Skip to content

Make 'browser_name' parameter indirect #284

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.

Sign up for GitHub

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Make 'browser_name' parameter indirect #284

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 5 additions & 2 deletions pytest-playwright-asyncio/pytest_playwright_asyncio/pytest_playwright.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ def delete_output_dir(pytestconfig: Any) -> None:
def pytest_generate_tests(metafunc: Any) -> None:
if "browser_name" in metafunc.fixturenames:
browsers = metafunc.config.option.browser or ["chromium"]
metafunc.parametrize("browser_name", browsers, scope="session")
metafunc.parametrize("browser_name", browsers, scope="session", indirect=True)
Comment on lines 92 to +95
Copy link

@MosZhao MosZhao Aug 8, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

If check the browser_name in metafuc.params the browser_name can support get browser from marker.parametrize of testcase. Such as the code:

Suggested change
def pytest_generate_tests(metafunc: Any) -> None:
if "browser_name" in metafunc.fixturenames:
browsers = metafunc.config.option.browser or ["chromium"]
metafunc.parametrize("browser_name", browsers, scope="session")
metafunc.parametrize("browser_name", browsers, scope="session", indirect=True)
def pytest_generate_tests(metafunc: Any) -> None:
if "browser_name" in metafunc.fixturenames:
browsers = metafunc.config.option.browser or ["chromium"]
if hasattr(metafunc, "params") and "browser_name" not in metafunc.params:
metafunc.parametrize("browser_name", browsers, scope="session")

And the fixture browser_name changes is not required.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think that, in this case, it should better be:

if "browser_name" not in getattr(metafunc, "params", {}):
    metafunc.parametrize("browser_name", browsers, scope="session")

Because, I've seen that "params" could not be present if not parameters have been defined yet and "brower_name" parametrization is not added then.

But, apart from that, how does this fix the problem? Tests are still wrongly sorted with this change:

pytest --browser chromium --browser firefox 

tests/gui_e2e/my_tests/test_a.py::test_aa[chromium-case_1] PASSED                                        [  6%]
tests/gui_e2e/my_tests/test_a.py::test_ab[chromium-case_1] PASSED                                        [ 12%]
tests/gui_e2e/my_tests/test_b.py::test_ba[chromium-case_1] PASSED                                        [ 18%]
tests/gui_e2e/my_tests/test_b.py::test_bb[chromium-case_1] PASSED                                        [ 25%]
tests/gui_e2e/my_tests/test_a.py::test_aa[chromium-case_2] PASSED                                        [ 31%]
tests/gui_e2e/my_tests/test_a.py::test_ab[chromium-case_2] PASSED                                        [ 37%]
tests/gui_e2e/my_tests/test_b.py::test_ba[chromium-case_2] PASSED                                        [ 43%]
tests/gui_e2e/my_tests/test_b.py::test_bb[chromium-case_2] PASSED                                        [ 50%]
tests/gui_e2e/my_tests/test_a.py::test_aa[firefox-case_1] PASSED                                         [ 56%]
tests/gui_e2e/my_tests/test_a.py::test_ab[firefox-case_1] PASSED                                         [ 62%]
tests/gui_e2e/my_tests/test_b.py::test_ba[firefox-case_1] PASSED                                         [ 68%]
tests/gui_e2e/my_tests/test_b.py::test_bb[firefox-case_1] PASSED                                         [ 75%]
tests/gui_e2e/my_tests/test_a.py::test_aa[firefox-case_2] PASSED                                         [ 81%]
tests/gui_e2e/my_tests/test_a.py::test_ab[firefox-case_2] PASSED                                         [ 87%]
tests/gui_e2e/my_tests/test_b.py::test_ba[firefox-case_2] PASSED                                         [ 93%]
tests/gui_e2e/my_tests/test_b.py::test_bb[firefox-case_2] PASSED                                         [100%]

Copy link

Choose a reason for hiding this comment

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

My suggestion does not keep the indirect=Ture.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I see, but then it does not fix the sorting problem

Copy link

Choose a reason for hiding this comment

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

how about impl, trylast=true?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Do you mean using this?

@pytest.hookimpl(trylast=True)
def pytest_generate_tests(metafunc: Any) -> None:
    if "browser_name" in metafunc.fixturenames:
        browsers = metafunc.config.option.browser or ["chromium"]
        metafunc.parametrize("browser_name", browsers, scope="session")

The order is even worse:

pytest --browser chromium --browser firefox 

my_tests/test_a.py::test_aa[case_1-chromium] PASSED                                        [  6%]
my_tests/test_a.py::test_ab[case_1-chromium] PASSED                                        [ 12%]
my_tests/test_b.py::test_ba[case_1-chromium] PASSED                                        [ 18%]
my_tests/test_b.py::test_bb[case_1-chromium] PASSED                                        [ 25%]
my_tests/test_a.py::test_aa[case_1-firefox] PASSED                                         [ 31%]
my_tests/test_a.py::test_ab[case_1-firefox] PASSED                                         [ 37%]
my_tests/test_b.py::test_ba[case_1-firefox] PASSED                                         [ 43%]
my_tests/test_b.py::test_bb[case_1-firefox] PASSED                                         [ 50%]
my_tests/test_a.py::test_aa[case_2-chromium] PASSED                                        [ 56%]
my_tests/test_a.py::test_ab[case_2-chromium] PASSED                                        [ 62%]
my_tests/test_b.py::test_ba[case_2-chromium] PASSED                                        [ 68%]
my_tests/test_b.py::test_bb[case_2-chromium] PASSED                                        [ 75%]
my_tests/test_a.py::test_aa[case_2-firefox] PASSED                                         [ 81%]
my_tests/test_a.py::test_ab[case_2-firefox] PASSED                                         [ 87%]
my_tests/test_b.py::test_ba[case_2-firefox] PASSED                                         [ 93%]
my_tests/test_b.py::test_bb[case_2-firefox] PASSED                                         [100%]

Copy link

@MosZhao MosZhao Aug 19, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Can you apply the changes about browser_name first? We need set browser_name list for part cases not all.

if "browser_name" not in getattr(metafunc, "params", {}):
    metafunc.parametrize("browser_name", browsers, scope="session")

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Do you mean this?

@pytest.hookimpl(trylast=True)
def pytest_generate_tests(metafunc: Any) -> None:
    if "browser_name" in metafunc.fixturenames and "browser_name" not in getattr(metafunc, "params", {}):
        browsers = metafunc.config.option.browser or ["chromium"]
        metafunc.parametrize("browser_name", browsers, scope="session")

It is the same than in the previous case:

pytest --browser chromium --browser firefox 

my_tests/test_a.py::test_aa[case_1-chromium] PASSED                                        [  6%]
my_tests/test_a.py::test_ab[case_1-chromium] PASSED                                        [ 12%]
my_tests/test_b.py::test_ba[case_1-chromium] PASSED                                        [ 18%]
my_tests/test_b.py::test_bb[case_1-chromium] PASSED                                        [ 25%]
my_tests/test_a.py::test_aa[case_1-firefox] PASSED                                         [ 31%]
my_tests/test_a.py::test_ab[case_1-firefox] PASSED                                         [ 37%]
my_tests/test_b.py::test_ba[case_1-firefox] PASSED                                         [ 43%]
my_tests/test_b.py::test_bb[case_1-firefox] PASSED                                         [ 50%]
my_tests/test_a.py::test_aa[case_2-chromium] PASSED                                        [ 56%]
my_tests/test_a.py::test_ab[case_2-chromium] PASSED                                        [ 62%]
my_tests/test_b.py::test_ba[case_2-chromium] PASSED                                        [ 68%]
my_tests/test_b.py::test_bb[case_2-chromium] PASSED                                        [ 75%]
my_tests/test_a.py::test_aa[case_2-firefox] PASSED                                         [ 81%]
my_tests/test_a.py::test_ab[case_2-firefox] PASSED                                         [ 87%]
my_tests/test_b.py::test_ba[case_2-firefox] PASSED                                         [ 93%]
my_tests/test_b.py::test_bb[case_2-firefox] PASSED                                         [100%]



def pytest_configure(config: Any) -> None:
Expand Down Expand Up @@ -389,7 +389,10 @@ def is_chromium(browser_name: str) -> bool:


@pytest.fixture(scope="session")
def browser_name(pytestconfig: Any) -> Optional[str]:
def browser_name(pytestconfig: Any, request: pytest.FixtureRequest) -> Optional[str]:
if hasattr(request, "param"):
return request.param

# When using unittest.TestCase it won't use pytest_generate_tests
# For that we still try to give the user a slightly less feature-rich experience
browser_names = pytestconfig.getoption("--browser")
Expand Down
7 changes: 5 additions & 2 deletions pytest-playwright/pytest_playwright/pytest_playwright.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ def delete_output_dir(pytestconfig: Any) -> None:
def pytest_generate_tests(metafunc: Any) -> None:
if "browser_name" in metafunc.fixturenames:
browsers = metafunc.config.option.browser or ["chromium"]
metafunc.parametrize("browser_name", browsers, scope="session")
metafunc.parametrize("browser_name", browsers, scope="session", indirect=True)


def pytest_configure(config: Any) -> None:
Expand Down Expand Up @@ -384,7 +384,10 @@ def is_chromium(browser_name: str) -> bool:


@pytest.fixture(scope="session")
def browser_name(pytestconfig: Any) -> Optional[str]:
def browser_name(pytestconfig: Any, request: pytest.FixtureRequest) -> Optional[str]:
if hasattr(request, "param"):
return request.param

# When using unittest.TestCase it won't use pytest_generate_tests
# For that we still try to give the user a slightly less feature-rich experience
browser_names = pytestconfig.getoption("--browser")
Expand Down
Loading