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

Enable pantsd in Travis #7440

Merged
merged 16 commits into from
Apr 15, 2019
Merged

Conversation

blorente
Copy link
Contributor

This is a WIP to iterate on the blockers to enable pantsd in Travis.

@blorente
Copy link
Contributor Author

Removing the Draft qualifier to trigger travis, but this is still not ready for review.

@blorente blorente marked this pull request as ready for review March 26, 2019 13:58
@blorente blorente force-pushed the blorente/pantsd-in-travis branch 4 times, most recently from ccb8c3e to 56ecae1 Compare March 28, 2019 17:52
@stuhood stuhood mentioned this pull request Apr 3, 2019
11 tasks
stuhood and others added 6 commits April 9, 2019 12:02
Also enable shutdown-after-use.

Add a decorator to disable pantsd in integration tests that fail under pantsd.

Blacklist a few more tests (which appear to be flaky, perhaps... gulp), and mark pantsd integration tests as hermetic.

Only apply pantsd on integration test shards.

Blacklist some other stuff

Fix test_stats_local_json_file

Add more reasoning for blacklisted tests

Skip some more tests

Add some colour to the hermetic errors

Also skip test_v2_list

Hacky way of making hermetic tests run

Import fixes

Skip some weird interactions with the hermetic hacks

Pointless Commit to trigger travis

Make stuff non-hermetic

Explicitly disable pantsd in the test coordinator

Add some color to some of the failures
@blorente
Copy link
Contributor Author

blorente commented Apr 9, 2019

I'm pretty happy with the state of this, removing the WIP tag, will blacklist more tomorrow if travis is not green. Some things to note:

  • I tried to separate the concepts of "the test coordinator is running pantsd" and "the integration tests spawn pantsd". This PR implements the latter, but whenever the former is necessary (we turn pantsd on by default), it should be isolated and should not require changes to this code.
  • The way I read the env var TRAVIS_AND_PANTS_INTEGRATION_TESTS_SHOULD_HAVE_PANTSD is a bit wonky, I'm totally open to suggestions.

@blorente blorente changed the title [WIP] Enable pantsd in Travis Enable pantsd in Travis Apr 9, 2019
Copy link
Sponsor Member

@stuhood stuhood left a comment

Choose a reason for hiding this comment

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

Thanks!

tests/python/pants_test/pants_run_integration_test.py Outdated Show resolved Hide resolved
@@ -284,6 +299,13 @@ def run_pants_with_workdir_without_waiting(self, command, workdir, config=None,
'--cache-bootstrap-read', '--cache-bootstrap-write'
])

if self.should_configure_pantsd():
args.append("--enable-pantsd=True")
args.append("--no-shutdown-pantsd-after-run")
Copy link
Sponsor Member

Choose a reason for hiding this comment

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

I thought that our intent right now was to actually enable --shutdown-pantsd-after-run... but I think that you also determined that because of the temporary workdir, pantsd will shutdown after each run anyway?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So we don't want to shutdown after every run inside the tests, because some of them rely on pantsd living run after run (although they don't rely on this codepath). We do want to set it (probably) in the overpants, the test coordinator, via travis env var. But this should be done at the same time as we enable pantsd by default, and shouldn't affect these changes at all.

Copy link
Sponsor Member

@stuhood stuhood Apr 9, 2019

Choose a reason for hiding this comment

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

I would argue that those tests should be marked hermetic and explicitly set the relevant values. But this works too!

Copy link
Contributor

@illicitonion illicitonion left a comment

Choose a reason for hiding this comment

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

Looks great :) Thanks!

Copy link
Contributor

@Eric-Arellano Eric-Arellano left a comment

Choose a reason for hiding this comment

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

Exciting!

Copy link
Contributor

@Eric-Arellano Eric-Arellano left a comment

Choose a reason for hiding this comment

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

Thanks for the renames! Beyond fixing the stage entry, this lgtm.

build-support/travis/travis.yml.mustache Outdated Show resolved Hide resolved
build-support/travis/travis.yml.mustache Outdated Show resolved Hide resolved
@blorente blorente merged commit 08fe94a into pantsbuild:master Apr 15, 2019
@stuhood
Copy link
Sponsor Member

stuhood commented Apr 15, 2019

Hooray! Thanks @blorente .

blorente added a commit that referenced this pull request Apr 16, 2019
**Problem**
The cron job was not configured to bootstrap the 3.6 version of pants, and therefore when we added the "3.6 + pantsd" shards in #7440, these don't work.

**Solution**
To add the 3.6 bootstrapping shards to the cron job.

**Result**
The cron job should now work, maybe?
blorente added a commit that referenced this pull request Apr 17, 2019
**Problem**

`travis-ci.sh` was removed in ef7f664 (#7548), but the changes to the cron shard in  #7440  were not rebased to reflect this update.
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.

4 participants