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: make conftest.py special with gazelle #879

Merged
merged 4 commits into from
Nov 10, 2022
Merged

Conversation

f0rmiga
Copy link
Collaborator

@f0rmiga f0rmiga commented Nov 9, 2022

conftest.py is a special file that should be used only with tests, so we create a py_library for it and add it as a dependency to py_test. By special-casing it, we take advantage of all the dependency resolution Gazelle offers. In contrast, when adding it to a py_test with # keep and excluding it from Gazelle, we also have to manage its dependencies manually.

Signed-off-by: Thulio Ferraz Assis <3149049+f0rmiga@users.noreply.github.com>
@f0rmiga f0rmiga requested a review from mattem November 9, 2022 19:20
@@ -0,0 +1,12 @@
import unittest
Copy link
Contributor

Choose a reason for hiding this comment

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

is the __test__.py file a gazelle convention that automatically becomes the test entrypoint, or is it generated? Reason I'm asking is this file name doesn't seem to match the allowed test patterns in generate, but gets included in the py_test sources.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It's special for Gazelle. The following is the main logic (though they are used in some other places).

pyTestEntrypointFilename = "__test__.py"
pyTestEntrypointTargetname = "__test__"

} else if !hasPyTestFile && f == pyTestEntrypointFilename {
hasPyTestFile = true

if hasPyTestFile {
// Only add the pyTestEntrypointFilename to the pyTestFilenames if
// the file exists on disk.
pyTestFilenames.Add(pyTestEntrypointFilename)
}

This allows the conftest.py to be used on sub-directories
as pytest would pick them up.

Signed-off-by: Thulio Ferraz Assis <3149049+f0rmiga@users.noreply.github.com>
Signed-off-by: Thulio Ferraz Assis <3149049+f0rmiga@users.noreply.github.com>
@f0rmiga f0rmiga changed the title fix: add conftest.py to py_test generated targets fix: make conftest.py special with gazelle Nov 9, 2022
@f0rmiga
Copy link
Collaborator Author

f0rmiga commented Nov 10, 2022

@hrfuller want to take another look? I changed the approach slightly since @mattem pointed out that it's good to have a separate py_library for it so we could add parent conftest.py files to a given py_test.

Signed-off-by: Thulio Ferraz Assis <3149049+f0rmiga@users.noreply.github.com>
@f0rmiga f0rmiga merged commit 912a505 into main Nov 10, 2022
@f0rmiga f0rmiga deleted the f0rmiga/gazelle-conftest branch November 10, 2022 18:43
@ewhauser
Copy link

Trying to upgrade from 0.13.0 to 0.14.0. I'm getting all sorts of resolution errors around existing tests, and this appears to be the culprit since it was the only Gazelle change in the release. Is it expected for this to be able to work with existing tests were conftest isn't in it's own library?

@f0rmiga
Copy link
Collaborator Author

f0rmiga commented Nov 16, 2022

@ewhauser I just got one of these myself. I'm not sure yet what causes it, but if I clear the BUILD file and ask Gazelle to generate it again, it does the right thing. I'll work on a fix.

@kersson
Copy link

kersson commented Jan 2, 2023

Hi @f0rmiga. Any progress on a fix? Minimally, I feel like this feature should be documented in the top-level README. Perhaps, also add a flag to disable it if you want to handle conftest yourself.

As a future feature idea, it would be great if the generated py_library(name="conftest") was automatically added as a dependency of all subpackages since that's how it works for pytest.

ianpegg-bc pushed a commit to ianpegg-bc/rules_python that referenced this pull request May 12, 2023
* fix: add conftest.py to py_test generated targets

Signed-off-by: Thulio Ferraz Assis <3149049+f0rmiga@users.noreply.github.com>

* fix: use separate py_library for conftest.py

This allows the conftest.py to be used on sub-directories
as pytest would pick them up.

Signed-off-by: Thulio Ferraz Assis <3149049+f0rmiga@users.noreply.github.com>

* fix: add testonly to conftest py_library

Signed-off-by: Thulio Ferraz Assis <3149049+f0rmiga@users.noreply.github.com>

* fix: testonly is a boolean, not a string

Signed-off-by: Thulio Ferraz Assis <3149049+f0rmiga@users.noreply.github.com>

Signed-off-by: Thulio Ferraz Assis <3149049+f0rmiga@users.noreply.github.com>
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.

None yet

5 participants