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

Ensure that target root order is preserved #4708

Merged
merged 3 commits into from Jun 27, 2017

Conversation

Projects
None yet
2 participants
@stuhood
Copy link
Member

stuhood commented Jun 27, 2017

Problem

The 1.3.0 stable branch contains an issue where target root order is not always preserved, which leads to ./pants paths $X $Y looking for paths from $Y to $X ~50% of the time. Apparently paths is (one of?) the only tasks that is order-dependent with regard to target roots. There are unit tests to validate that roots are ordered correctly, but due to #4401 they are not all providing coverage yet.

This bug does not actually exist in master (because it was fixed in #4679), but while looking for the bug I noticed that the relevant code could use further cleanup, which will be cherry-picked to the 1.3.0 branch.

Solution

Add integration test coverage for paths, with a comment to remove it in favor of the unit tests once #4401 lands.

@stuhood stuhood added this to the 1.3.1 milestone Jun 27, 2017

@stuhood stuhood requested review from baroquebobcat , benjyw and kwlzn Jun 27, 2017

@baroquebobcat
Copy link
Contributor

baroquebobcat left a comment

Makes sense to me. Thanks for fixing this!

@@ -0,0 +1,26 @@
# coding=utf-8
# Copyright 2015 Pants project contributors (see CONTRIBUTORS.md).

This comment has been minimized.

@baroquebobcat

baroquebobcat Jun 27, 2017

Contributor

nit: date

@stuhood stuhood merged commit acfb6f8 into pantsbuild:master Jun 27, 2017

1 check was pending

continuous-integration/travis-ci/pr The Travis CI build is in progress
Details

@stuhood stuhood deleted the twitter:stuhood/preserve-target-root-order branch Jun 27, 2017

stuhood added a commit that referenced this pull request Jun 27, 2017

Ensure that target root order is preserved (#4708)
The 1.3.0 stable branch contains an issue where target root order is not always preserved, which leads to `./pants paths $X $Y` looking for paths from `$Y` to `$X` ~50% of the time. Apparently `paths` is (one of?) the only tasks that is order-dependent with regard to target roots. There are unit tests to validate that roots are ordered correctly, but due to #4401 they are not all providing coverage yet.

This bug does not actually exist in master (because it was fixed in #4679), but while looking for the bug I noticed that the relevant code could use further cleanup, which will be cherry-picked to the `1.3.0` branch.

Add integration test coverage for `paths`, with a comment to remove it in favor of the unit tests once #4401 lands.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment