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

skip integration test with pants_requirement() #6493

Conversation

cosmicexplorer
Copy link
Contributor

Problem

See #6490. Currently, test_pants_requirement_setup_requires_version requires the version of pants specified in src/python/pants/VERSION to have been released to pypi. This makes it somewhat difficult for release candidates to pass CI.

Solution

  • Skip test_pants_requirement_setup_requires_version to unbreak release CI. This is the only references to the pants_setup_requires target I can find in the repo.

Result

I don't necessarily think that depending on a pants_requirement() in a python_dist()'s setup_requires needs to be tested in CI, and I haven't figured out whether it's appropriate to delete this test entirely. Either way, that will be figured out in a followup diff.

currently, this test passing requires the version of pants specified in `src/python/pants/VERSION`
to have been released to pypi. this makes it difficult for release candidates to pass testing.
Copy link
Member

@jsirois jsirois left a comment

Choose a reason for hiding this comment

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

Thanks Danny.

@cosmicexplorer
Copy link
Contributor Author

Had to restart a couple of integration test shards due to travis timeouts during some pantsd testing (which I just barely forgot to keep the logs for). Will watch if the timeout occurs again.

@cosmicexplorer cosmicexplorer merged commit fce01b9 into pantsbuild:master Sep 12, 2018
cosmicexplorer added a commit that referenced this pull request Sep 12, 2018
### Problem

Spent a bit more time thinking after #6493 and there's no reason we need to test the case of a `pants_requirement()` in a `python_dist()`'s `setup_requires` -- as @jsirois said in #6490, these are both well-tested at lower levels, and the increase in CI time alone, even if it worked, is enough to make this removal-worthy.

### Solution

- Delete `examples/src/python/example/python_distribution/hello/pants_setup_requires/` and `test_pants_requirement_setup_requires_version`, which is the broken test that depends on it. 

### Result

This was intentionally separated from #6493 so we can have absolutely no fear of merging that first to fix CI. Resolves #6490.
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

2 participants