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

Parametrized python_tests and python_sources in same package not lining up when using explicit dependencies #20739

Closed
ndellosa95 opened this issue Apr 2, 2024 · 5 comments · Fixed by #20768
Assignees
Labels
backend: Python Python backend-related issues bug

Comments

@ndellosa95
Copy link
Contributor

Describe the bug
I have a python_source target in the same directory as a python_test target. Both are parametrized for multiple resolves/environments/interpreter constraints. The python_test target in question does not import the python_source target but instead calls it via the subprocess module, so I have the python_source target as an explicit dependency of the python_test one. This is causing an error because the python_test target is grabbing the first parametrization as the dependency instead of the matching one. pants peek output below:

{
    "address": "my/python/tests/test_the_thing.py@parametrize=py311",
    "target_type": "python_test",
    "batch_compatibility_tag": null,
    "dependencies": [
      "my/inferred_dep/__init__.py@parametrize=py311",
      "my/python/tests/__init__.py:sources_for_tests@parametrize=py38",
      "my/python/tests/source_for_test.py:sources_for_tests@parametrize=py38"
    ],
    "dependencies_raw": [
      ":sources_for_tests"
    ],
    "description": null,
    "environment": "py311-test",
    "extra_env_vars": null,
    "interpreter_constraints": [
      "==3.11.*"
    ],
    "resolve": "py311",
    "run_goal_use_sandbox": null,
    "runtime_package_dependencies": null,
    "skip_black": false,
    "skip_docformatter": false,
    "skip_isort": false,
    "skip_mypy": true,
    "skip_ruff": true,
    "skip_tests": false,
    "source_raw": "test_the_thing.py",
    "sources": [
      "my/python/tests/test_the_thing.py"
    ],
    "sources_fingerprint": "b1274e36d37387cc0fca9f18cd77e456483663e6ac869ec1ecce9aaa502a8d83",
    "tags": null,
    "timeout": null,
    "xdist_concurrency": null
 }

Pants version
2.19.1

OS
Intel Mac

@ndellosa95 ndellosa95 added the bug label Apr 2, 2024
@huonw huonw added the backend: Python Python backend-related issues label Apr 2, 2024
@huonw
Copy link
Contributor

huonw commented Apr 2, 2024

Sorry for the trouble; can you create a reduced example as a demonstration?

@kaos
Copy link
Member

kaos commented Apr 8, 2024

I understand the issue here.. will see if I can come up with a fix for it.

@kaos kaos self-assigned this Apr 8, 2024
@kaos
Copy link
Member

kaos commented Apr 8, 2024

@ndellosa95 Can you share the relevant parts of your BUILD files for this (redacted as needed), just to make sure I test the right thing? Thanks :)

@kaos
Copy link
Member

kaos commented Apr 8, 2024

Or, if you can confirm that this test matches what you have..

    assert_generated(
        generated_targets_rule_runner,
        Address("demo", target_name="tst"),
        dedent(
            """\
            generator(
              name='src',
              sources=['src1.ext'],
              **parametrize('b1', resolve='a'),
              **parametrize('b2', resolve='b'),
            )
            generator(
              name='tst',
              sources=['tst1.ext'],
              dependencies=['./src1.ext:src'],
              **parametrize('b1', resolve='a'),
              **parametrize('b2', resolve='b'),
            )
            """
        ),
        ["src1.ext", "tst1.ext"],
        expected_dependencies={
            "demo/src1.ext:src@parametrize=b1": set(),
            "demo/src1.ext:src@parametrize=b2": set(),
            "demo/tst1.ext:tst@parametrize=b1": {
                "demo/src1.ext:src@parametrize=b1",
            },
            "demo/tst1.ext:tst@parametrize=b2": {
                "demo/src1.ext:src@parametrize=b2",
            },
            "demo:src@parametrize=b1": {
                "demo/src1.ext:src@parametrize=b1",
            },
            "demo:src@parametrize=b2": {
                "demo/src1.ext:src@parametrize=b2",
            },
            "demo:tst@parametrize=b1": {
                "demo/tst1.ext:tsr@parametrize=b1",
            },
            "demo:tst@parametrize=b2": {
                "demo/tst1.ext:tst@parametrize=b2",
            },
        },
    )

which does fail with a missmatched parametrization for the explicitly provided dependency:

E               'demo/tst1.ext:tst@parametriz    'demo/tst1.ext:tst@parametriz
E             e=b2': set([                     e=b2': set([
E                 'demo/src1.ext:src@parametr      'demo/src1.ext:src@parametr
E             ize=b1',                         ize=b2',
E               ]),                              ]),

@ndellosa95
Copy link
Contributor Author

Or, if you can confirm that this test matches what you have..

    assert_generated(
        generated_targets_rule_runner,
        Address("demo", target_name="tst"),
        dedent(
            """\
            generator(
              name='src',
              sources=['src1.ext'],
              **parametrize('b1', resolve='a'),
              **parametrize('b2', resolve='b'),
            )
            generator(
              name='tst',
              sources=['tst1.ext'],
              dependencies=['./src1.ext:src'],
              **parametrize('b1', resolve='a'),
              **parametrize('b2', resolve='b'),
            )
            """
        ),
        ["src1.ext", "tst1.ext"],
        expected_dependencies={
            "demo/src1.ext:src@parametrize=b1": set(),
            "demo/src1.ext:src@parametrize=b2": set(),
            "demo/tst1.ext:tst@parametrize=b1": {
                "demo/src1.ext:src@parametrize=b1",
            },
            "demo/tst1.ext:tst@parametrize=b2": {
                "demo/src1.ext:src@parametrize=b2",
            },
            "demo:src@parametrize=b1": {
                "demo/src1.ext:src@parametrize=b1",
            },
            "demo:src@parametrize=b2": {
                "demo/src1.ext:src@parametrize=b2",
            },
            "demo:tst@parametrize=b1": {
                "demo/tst1.ext:tsr@parametrize=b1",
            },
            "demo:tst@parametrize=b2": {
                "demo/tst1.ext:tst@parametrize=b2",
            },
        },
    )

which does fail with a missmatched parametrization for the explicitly provided dependency:

E               'demo/tst1.ext:tst@parametriz    'demo/tst1.ext:tst@parametriz
E             e=b2': set([                     e=b2': set([
E                 'demo/src1.ext:src@parametr      'demo/src1.ext:src@parametr
E             ize=b1',                         ize=b2',
E               ]),                              ]),

Yes, it always selects the first target in the parametrization.

kaos added a commit that referenced this issue Apr 12, 2024
…0768)

When both a target and a explicitly provided dependency is using
parametrize groups (the `**parametrize(..)` syntax), and the dependency
does not explicitly provide the parametrization to use, pick the
matching parametrization group with the target if available, rather than
just the first one.

Closes #20739
WorkerPants pushed a commit that referenced this issue Apr 12, 2024
…0768)

When both a target and a explicitly provided dependency is using
parametrize groups (the `**parametrize(..)` syntax), and the dependency
does not explicitly provide the parametrization to use, pick the
matching parametrization group with the target if available, rather than
just the first one.

Closes #20739
WorkerPants pushed a commit that referenced this issue Apr 12, 2024
…0768)

When both a target and a explicitly provided dependency is using
parametrize groups (the `**parametrize(..)` syntax), and the dependency
does not explicitly provide the parametrization to use, pick the
matching parametrization group with the target if available, rather than
just the first one.

Closes #20739
kaos added a commit that referenced this issue Apr 15, 2024
…erry-pick of #20768) (#20778)

When both a target and a explicitly provided dependency is using
parametrize groups (the `**parametrize(..)` syntax), and the dependency
does not explicitly provide the parametrization to use, pick the
matching parametrization group with the target if available, rather than
just the first one.

Closes #20739

Co-authored-by: Andreas Stenius <[email protected]>
kaos added a commit that referenced this issue Apr 17, 2024
…erry-pick of #20768) (#20779)

When both a target and a explicitly provided dependency is using
parametrize groups (the `**parametrize(..)` syntax), and the dependency
does not explicitly provide the parametrization to use, pick the
matching parametrization group with the target if available, rather than
just the first one.

Closes #20739

Co-authored-by: Andreas Stenius <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backend: Python Python backend-related issues bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants