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

ENH Add rename_fixture and standalone decorators #101

Merged
merged 2 commits into from
Aug 7, 2023

Conversation

hoodmane
Copy link
Member

We use a bunch of different selenium variants but for the most part they are drop-in replacements for each other. For instance, when using selenium_standalone, we always just say selenium = selenium_standalone at the top of the function. I think it's clearer if we use a decorator to rename the fixture.

We use a bunch of different selenium variants but for the most part they are
drop-in replacements for each other. For instance, when using `selenium_standalone`,
we always just say `selenium = selenium_standalone` at the top of the function. I
think it's clearer if we use a decorator to rename the fixture.
@ryanking13
Copy link
Member

Thanks @hoomane! We have some logic for modifying the test order when standalone fixtures are used in the Safari browser. Could you check if it works well with rename_fixture?

standalone_fixtures = [
"selenium_standalone",
"selenium_standalone_noload",
"selenium_webworker_standalone",
]
def _has_standalone_fixture(fixturenames):
for fixture in fixturenames:
if fixture in standalone_fixtures:
return True
return False
def _get_item_position(item):
counter[0] += 1
if any(
[re.match(r"^safari[\-$]?", el) for el in item.keywords._markers.keys()]
) and _has_standalone_fixture(item._request.fixturenames):
return counter[0] - OFFSET
return counter[0]

different selenium variants

BTW, we have both selenium and playwright runners so I think someday we need to change the fixture name.

@hoodmane
Copy link
Member Author

Sure. It should work fine with everything since the rename happens right away in the decorator nothing has the opportunity to see the original function,

Copy link
Member

@ryanking13 ryanking13 left a comment

Choose a reason for hiding this comment

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

Thanks for the check!

There are some CI failures, but I think those are not relevant to this PR (some are about hypothesis, and some are about playwright)...

@hoodmane hoodmane merged commit 4f9c154 into pyodide:main Aug 7, 2023
33 of 41 checks passed
@hoodmane hoodmane deleted the rename_fixture branch August 7, 2023 13:05
hoodmane pushed a commit that referenced this pull request Aug 9, 2023
This fixes a test failure that started to occur after #101.

```
! _pytest.outcomes.Exit: playwright failed to launch
```

The cause of the problem was that the combination of the `pytester` fixture and the `pytest_configure` hook is not very good.
We use `pytest_configure` to manage global settings, but this function is called again when we run sub-tests using pytester, resulting in new global settings being overwritten.

So I added a check to prevent overwriting global settings once it is set.
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