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

Remove backend/python/tasks tests from Python 3 CI blacklist and refactor their BUILD entries to be more granular #7463

Merged
merged 3 commits into from Mar 31, 2019

Conversation

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

commented Mar 30, 2019

Problem

Currently, you can only run ./pants test tests/python/pants_test/backend/python/tasks:tasks or ./pants test tests/python/pants_test/backend/python/tasks:integration.

This lack of granularity makes it difficult to develop in this folder, as the former has a test timeout of 10 minutes and the latter of 40 minutes. While you can use ./pants test.pytest -- -k test_name, this does not get the specific file but rather the names of individual tests.

It also makes it harder to tell which tests are causing the long timeouts for this folder.

Solution

Each test file gets its own BUILD entry, as we do in most folders.

Also remove tests from Py3 blacklist

All tests now pass with Python 3. This is hypothesized to be thanks to:

  1. Performance improvement thanks to #7447. As we recall, some of the tests were timing out, and this performance fix reduces the risk of timeouts.
  2. Skipping Pexrc-related tests thanks to #7285.

Note all integration tests were ran four times in CI to check for flakiness.

Also refactor test_python_run_integration.py's interpreter selection

Deduplicate the constraints and update the constraints to reflect our modern interpreter usage. For example, currently Py37 may not be chosen, which means our cron job would potentially fail.

@Eric-Arellano Eric-Arellano changed the title WIP: Refactor backend/python/tasks tests BUILD entries for more granular targets Refactor backend/python/tasks tests BUILD entries for more granular targets and remove from Python 3 failure Mar 30, 2019

@Eric-Arellano Eric-Arellano changed the title Refactor backend/python/tasks tests BUILD entries for more granular targets and remove from Python 3 failure Refactor backend/python/tasks tests BUILD entries for more granular targets and remove from Python 3 CI blacklist Mar 30, 2019

@Eric-Arellano Eric-Arellano changed the title Refactor backend/python/tasks tests BUILD entries for more granular targets and remove from Python 3 CI blacklist Refactor backend/python/tasks tests BUILD entries for more granular targets and remove them from Python 3 CI blacklist Mar 30, 2019

@Eric-Arellano Eric-Arellano changed the title Refactor backend/python/tasks tests BUILD entries for more granular targets and remove them from Python 3 CI blacklist Remove backend/python/tasks tests from Python 3 CI blacklist and refactor their BUILD entries to be more granular Mar 30, 2019

@cosmicexplorer

This comment has been minimized.

Copy link
Contributor

commented Mar 31, 2019

./pants test tests/python/pants_test/backend/python/tasks:

Is this the exact command line you ran? Because I'm pretty sure the trailing : globs all targets in the directory, and would pull in :integration as well (zero trailing colons uses the singular default target for the directory), but you mentioned it only took 10 minutes compared to :integration alone.

@Eric-Arellano

This comment has been minimized.

Copy link
Contributor Author

commented Mar 31, 2019

./pants test tests/python/pants_test/backend/python/tasks:

Is this the exact command line you ran?

No, I meant to say ./pants test tests/python/pants_test/backend/python/tasks:tasks. Updated to reflect this.

Also the times mentioned are the timeouts specified in the BUILD, not how long it actually takes necessarily.

'tests/python/pants_test/testutils:py2_compat',
],
tags = {'integration'},
timeout=450

This comment has been minimized.

Copy link
@jsirois

jsirois Mar 31, 2019

Member

This is horrific. It's true the LHS is even worse, which is ~incomprehensible. I look forward to the day where we can kill all timeout arguments in our BUILD files.

This comment has been minimized.

Copy link
@Eric-Arellano

Eric-Arellano Mar 31, 2019

Author Contributor

Agreed :/ Part of the motivation for this PR is to call attention to which tests in particular take a long time.

@Eric-Arellano Eric-Arellano merged commit a995cbe into pantsbuild:master Mar 31, 2019

1 check passed

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

@Eric-Arellano Eric-Arellano deleted the Eric-Arellano:backend-python-tests branch Mar 31, 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.