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

use a fixture for pylint_plugins test #5849

Merged
merged 3 commits into from
Dec 28, 2022
Merged

Conversation

cognifloyd
Copy link
Member

@cognifloyd cognifloyd commented Dec 16, 2022

Background

This is another part of introducing pants, as discussed in various TSC meetings.

Related PRs can be found in:

Overview of this PR

In #5848, I will add a GHA workflow to run some tests via pants, starting with tests in pylint_plugins/. But, the current tests in pylint_plugins import from st2common because that was a quick way to throw together the tests. This is a problem because pylint_plugins/ and st2* each use different resolves (and therefore separate lockfiles). That means that pants puts together the sandbox for running the pylint_plugins tests, it will not include the st2* modules.

Note that running pylint ON our source code works just fine. In that case, pants knows we need pylint + any source plugins for pylint + the source code that pylint needs to inspect. For tests, however, everything is based on dependencies between targets. I could use a pants feature that puts our code in both the st2 and the pylint_plugins resolve, but that makes the lockfile and BUILD file management more complex.

So, I copied a few bits from st2common into a test fixture for the pylint_plugins tests. The Makefile runs the pylint_plugins tests just before it runs pylint, so look in the "CI / Compile" jobs to see the test results (which pass). I've also tested with pants in #5848 to make sure this change works for both ways to test this.

@cognifloyd cognifloyd added this to the pants milestone Dec 16, 2022
@cognifloyd cognifloyd self-assigned this Dec 16, 2022
@pull-request-size pull-request-size bot added the size/L PR that changes 100-499 lines. Requires some effort to review. label Dec 16, 2022
@cognifloyd cognifloyd force-pushed the pants-pylint_plugins_test branch 3 times, most recently from 31810fe to c8d6715 Compare December 16, 2022 20:20
@cognifloyd cognifloyd marked this pull request as ready for review December 16, 2022 20:42
@cognifloyd cognifloyd enabled auto-merge (squash) December 16, 2022 20:42
Copy link
Contributor

@amanda11 amanda11 left a comment

Choose a reason for hiding this comment

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

See comment mentioned.

pylint_plugins/fixtures/api_models.py Outdated Show resolved Hide resolved
@cognifloyd cognifloyd requested a review from a team December 19, 2022 20:44
@cognifloyd cognifloyd merged commit 0604a16 into master Dec 28, 2022
@cognifloyd cognifloyd deleted the pants-pylint_plugins_test branch December 28, 2022 22:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pantsbuild size/L PR that changes 100-499 lines. Requires some effort to review. tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants