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

Move V2 test runner integration test into proper location of `backend/python` folder #7847

Merged
merged 3 commits into from Jun 5, 2019

Conversation

Projects
None yet
3 participants
@Eric-Arellano
Copy link
Contributor

commented Jun 4, 2019

test_test_integration.py was really an integration test for the V2 Pytest runner, not for the generic V2 test runner. For example, these tests would not make sense in the context of running Java tests. Related code should live near each other to make things more discoverable.

Also, this changes the BUILD for pants_test/rules to create an entry for each test file and to properly include each test's dependencies, which were quite out of date. Having a target per test file makes it easier to keep dependencies up to date and makes it easier to run a specific test suite.

This is prework for #7846.

@Eric-Arellano Eric-Arellano requested review from stuhood and illicitonion Jun 4, 2019

@jsirois

jsirois approved these changes Jun 4, 2019

@@ -3,7 +3,7 @@

python_tests(
name='inject_init',
sources=['test_inject_init.py'],

This comment has been minimized.

Copy link
@jsirois

jsirois Jun 4, 2019

Member

My read on this affordance is that it supports legacy targets and the hope is for the irregularity to go away:

  1. # The source argument is automatically promoted to the sources argument, and rules should
    # always use the sources argument; don't error if people used source in their BUILD file.
    # There doesn't appear to be an easy way to error if a Target subclass explicitly takes
    # `source` as a named kwarg; it would be nice to error rather than silently swallow the value,
    # if there were such a way.
    ignore_params.add('source')
  2. source attribute is automatically promoted to sources #5908
  3. Undocumented feature: https://www.pantsbuild.org/build_dictionary.html

My read could definitely be wrong though - @illicitonion seems like the expert here on intent.

This comment has been minimized.

Copy link
@illicitonion

illicitonion Jun 4, 2019

Contributor

I'm largely indifferent to source vs sources; both work, there are guard-rails in place to stop you from using both, and I have no intention of trying to get rid of either...

@@ -3,7 +3,7 @@

python_tests(
name='inject_init',
sources=['test_inject_init.py'],

This comment has been minimized.

Copy link
@illicitonion

illicitonion Jun 4, 2019

Contributor

I'm largely indifferent to source vs sources; both work, there are guard-rails in place to stop you from using both, and I have no intention of trying to get rid of either...

@Eric-Arellano Eric-Arellano changed the title Move V2 test runner integration into proper location of `backend/python` folder Move V2 test runner integration test into proper location of `backend/python` folder Jun 5, 2019

@Eric-Arellano Eric-Arellano merged commit 37aa646 into pantsbuild:master Jun 5, 2019

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@Eric-Arellano Eric-Arellano deleted the Eric-Arellano:v2-move-tests branch Jun 5, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.