Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(regression-tests): correct path for acl files #1829

Merged
merged 1 commit into from
Sep 9, 2023
Merged

Conversation

kostasrim
Copy link
Contributor

@kostasrim kostasrim commented Sep 9, 2023

The issue was that we appended the aclfile name onto the dragonfly binary (path + binary name). This ofc would break

@kostasrim
Copy link
Contributor Author

kostasrim commented Sep 9, 2023

@kostasrim kostasrim self-assigned this Sep 9, 2023
@kostasrim kostasrim enabled auto-merge (squash) September 9, 2023 08:45
Comment on lines 249 to +254
@pytest.mark.asyncio
@dfly_args(
{
"aclfile": os.environ.get(
"DRAGONFLY_PATH/ok.acl", os.path.join(SCRIPTS_DIR, "../../build-dbg/ok.acl")
),
"port": 1111,
}
)
@dfly_args({"port": 1111})
async def test_good_acl_file(df_local_factory):
df = df_local_factory.create()
path = df_local_factory.params.path
acl = os.path.join(os.path.dirname(path), "ok.acl")
df = df_local_factory.create(aclfile=acl)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See how easier it would've been to place them just locally 😝

And now you use @dfly_args with a random port to move the default instance out of the way

Copy link
Contributor Author

@kostasrim kostasrim Sep 9, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See how easier it would've been to place them just locally

That's exactly what you suggested 🤣

And now you use @dfly_args with a random port to move the default instance out of the way

I will patch this on the next PR, I will make changes anyway so I didn;t bother

@kostasrim kostasrim merged commit fc6b7e5 into main Sep 9, 2023
@kostasrim kostasrim deleted the fix_reg branch September 9, 2023 08:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants