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

Narrow the scope of test_v2_list to a well-known static set of targets #6688

Merged
merged 2 commits into from Oct 29, 2018

Conversation

Projects
None yet
2 participants
@blorente
Contributor

blorente commented Oct 26, 2018

Problem

As described in #6687, test_v2_list is now flaky. This is due to the way pants invocations are done, in a temporary workdir whose contents might change from one run to the other.

Solution

The solution is to narrow down the scope of the tests to a well-known, unlikely to change set of targets (3rdparty in this case).

Result

Hopefully that test should be deterministically successful now. This fixes #6687

@blorente blorente changed the title from Narrow the scope of the tests to a well-known static set of targets to Narrow the scope of test_v2_list to a well-known static set of targets Oct 26, 2018

@stuhood

This comment has been minimized.

Member

stuhood commented Oct 26, 2018

Oh, hm. I don't think I realized that you had changed the test in #6516 to try to compare the --v1 and --v2 list commands. Those are not expected to be equal (so it's very surprising to me that they would ever be equal).

The v1 codepath consumes the Target.compute_injectable_specs and Target.compute_dependency_specs properties, but the v2 codepath will/should not.

I'd recommend moving the test back to not attempting to compare --v1 to --v2.

Sorry that I missed that in review!

@blorente

This comment has been minimized.

Contributor

blorente commented Oct 29, 2018

I reverted the tests to what they were before #6516, so that now we have the plain test_v2_list we had before, and another one that just checks that caching doesn't happen.

@stuhood

Thanks!

@stuhood stuhood merged commit c4be974 into pantsbuild:master Oct 29, 2018

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment