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

Test commit: inspect the env. vars. Travis uses. #2309

Closed
wants to merge 2 commits into from

Conversation

dhermes
Copy link
Contributor

@dhermes dhermes commented Sep 12, 2016

This is sniffing around towards #2277, to see how much info we can use to restrict the tests we run.


Context: I tried to build a similar feature in late 2014 / early 2015 and the env. vars. weren't very useful. Hoping that has changed in the last two years.

@dhermes dhermes added the do not merge Indicates a pull request not ready for merge, due to either quality or timing. label Sep 12, 2016
@dhermes dhermes self-assigned this Sep 12, 2016
@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Sep 12, 2016
@dhermes
Copy link
Contributor Author

dhermes commented Sep 12, 2016

FWIW I was expecting these vars and got all of them but TRAVIS_TEST_RESULT:

TRAVIS: true
TRAVIS_BUILD_DIR: /home/travis/build/GoogleCloudPlatform/google-cloud-python
TRAVIS_LANGUAGE: python
TRAVIS_OS_NAME: linux
TRAVIS_REPO_SLUG: GoogleCloudPlatform/google-cloud-python
TRAVIS_TAG:
TRAVIS_PYTHON_VERSION: 2.7

# These ones seem to change with each PR / push
TRAVIS_SECURE_ENV_VARS: false
TRAVIS_BRANCH: master
TRAVIS_EVENT_TYPE: pull_request
TRAVIS_PULL_REQUEST: 2309

# These ones seem to change with each build
TRAVIS_BUILD_ID: 159449750
TRAVIS_BUILD_NUMBER: 4806
TRAVIS_COMMIT: 07128f1ff0a3930c837728e39d9453984a75efbb
TRAVIS_COMMIT_RANGE: 1537d4a74be261c3f30eeed64b37d5dcdd3fba17...d038c32b438c753128945001e2b4e05b8742d312
TRAVIS_JOB_ID: 159449751
TRAVIS_JOB_NUMBER: 4806.1

From the build lifecycle doc, it seems that TRAVIS_TEST_RESULT is only populated in the after_success / after_failure / after_script environments.

@dhermes
Copy link
Contributor Author

dhermes commented Sep 13, 2016

TRAVIS_COMMIT (07128f1) seems to be a temporary merge commit of HEAD in this branch merged into HEAD of master, and those are the first and last (respectively) in TRAVIS_COMMIT_RANGE

@dhermes dhermes force-pushed the dont-merge-2277-sleuthing branch from d038c32 to dfbf929 Compare September 13, 2016 01:37
@dhermes
Copy link
Contributor Author

dhermes commented Sep 13, 2016

I rebased on top master (#2305 was merged) and adding another commit on top to see how it changes TRAVIS_COMMIT_RANGE

@dhermes
Copy link
Contributor Author

dhermes commented Sep 13, 2016

In the latest build, the only env. var. changes were:

TRAVIS_BUILD_ID: 159472570
TRAVIS_BUILD_NUMBER: 4810
TRAVIS_COMMIT: c9e955423f57e99a6be2fda7e4b73ff1a9f5beac
TRAVIS_COMMIT_RANGE: 397014a17d80225526b86d5894780e1544a67175...dfbf929a6ca8426c6afeeccd2e587a34d65cb3e7
TRAVIS_JOB_ID: 159472571
TRAVIS_JOB_NUMBER: 4810.1

After the rebase the HEAD commit in master moved to 397014a and the 2 commits in this PR became:

  1. b6f6ed4
  2. dfbf929

As before c9e955423f57e99a6be2fda7e4b73ff1a9f5beac is a hidden commit that merges my branch into HEAD (i.e. dfbf929 into 397014a)

@dhermes
Copy link
Contributor Author

dhermes commented Sep 13, 2016

From How-Pull-Requests-are-Tested

Rather than test the commits that have been pushed to the branch the pull request is from, we test the merge between the origin and the upstream branch. To only build on push events, you can disable Build on Pull Requests from your repository settings....We rely on the merge commit that GitHub transparently creates between the changes in the source branch and the upstream branch the pull request is sent against.

@dhermes dhermes force-pushed the dont-merge-2277-sleuthing branch from dfbf929 to 7147a65 Compare September 13, 2016 18:40
@dhermes
Copy link
Contributor Author

dhermes commented Sep 13, 2016

@dhermes dhermes force-pushed the dont-merge-2277-sleuthing branch from 7147a65 to c274f02 Compare September 13, 2016 20:23
@dhermes
Copy link
Contributor Author

dhermes commented Sep 14, 2016

ENV VARS: In the pull request build the usual players changed. In the "push" build, there are some differences:

# These ones seem to change with each PR / push
TRAVIS_SECURE_ENV_VARS: true               # vs. false
TRAVIS_BRANCH: dont-merge-2277-sleuthing   # vs. master
TRAVIS_EVENT_TYPE: push                    # vs. pull_request
TRAVIS_PULL_REQUEST: false                 # vs. 2309

Also TRAVIS_COMMIT is the actual commit at the tip of the branch (i.e. the one that was pushed, c274f02) and TRAVIS_COMMIT_RANGE covers a range from an invalid commit to a valid one. The beginning commit (7147a65) isn't in the branch history: it's from the previous "push" build, but I force-pushed over it. The 2nd (c274f02) is just the final commit in that branch.

Also, the commit hashes in TRAVIS_COMMIT_RANGE are 12 chars instead of the full 40 as we've seen in pull request builds (this is also different than the --pretty=%h length, which is 7).


As for the state of the git checkout in each, the PR is checked out at FETCH_HEAD which is just the result of running

$ git fetch origin +refs/pull/2309/merge:
$ git checkout -qf FETCH_HEAD

while the "push" build is checked out at a detached HEAD for the lead commit in the branch (c274f02). This is done explicitly

$ git checkout -qf c274f02461c82627cc10a71185df46e6091f97a8

In either case, there is only one git remote (I was curious in a pull request if the originating fork could be found anywhere).

Finally, a limited checkout is in place, making sure master is the only fetched branch in the PR build and dont-merge-2277-sleuthing is the only branch in the "push" build:

$ git clone --depth=50                                    ...
$ git clone --depth=50 --branch=dont-merge-2277-sleuthing ...

@dhermes dhermes force-pushed the dont-merge-2277-sleuthing branch from c274f02 to 4a52ba7 Compare September 22, 2016 19:35
@dhermes dhermes force-pushed the dont-merge-2277-sleuthing branch from 4a52ba7 to 8708169 Compare October 4, 2016 23:28
@dhermes
Copy link
Contributor Author

dhermes commented Oct 4, 2016

It looks like some new vars like TRAVIS_PULL_REQUEST_BRANCH and TRAVIS_PULL_REQUEST_SHA have been added so I triggered a new build. (Oops)

From that build we see the new variables as expected

# NEW!
TRAVIS_PULL_REQUEST_BRANCH: dont-merge-2277-sleuthing
TRAVIS_PULL_REQUEST_SHA: d4bc9d575e95146071b9892edf0b467a91f49f4c

d4bc9d5 is the HEAD commit in the branch, which agrees with the end commit in TRAVIS_COMMIT_RANGE and distinguishes from the target GitHub merge commit in TRAVIS_COMMIT (bc0a24f).

@dhermes
Copy link
Contributor Author

dhermes commented Oct 4, 2016

I just pushed a tag to trigger a tag build. Have already deleted tag, but the build remains.

The non-trivial changes we see are as follows

NEW (TAG)                       | OLD (PR)
--------------------------------|------------------------------------------------------------------
TRAVIS_TAG: dont-tag-sleuth     | TRAVIS_TAG:
TRAVIS_BRANCH: dont-tag-sleuth  | TRAVIS_BRANCH: master
TRAVIS_EVENT_TYPE: push         | TRAVIS_EVENT_TYPE: pull_request
TRAVIS_PULL_REQUEST: false      | TRAVIS_PULL_REQUEST: 2309
TRAVIS_PULL_REQUEST_BRANCH:     | TRAVIS_PULL_REQUEST_BRANCH: dont-merge-2277-sleuthing
TRAVIS_PULL_REQUEST_SHA:        | TRAVIS_PULL_REQUEST_SHA: d4bc9d575e95146071b9892edf0b467a91f49f4c

  • It's not surprising to see TRAVIS_TAG get populated nor is it surprising to see TRAVIS_PULL_REQUEST be false or TRAVIS_PULL_REQUEST_BRANCH / TRAVIS_PULL_REQUEST_SHA be empty
  • It is somewhat surprising that TRAVIS_EVENT_TYPE is push rather than something more specialized (like tag)
  • It IS surprising that the TRAVIS_BRANCH is given, since a tag doesn't have any real branch

This is sniffing around towards googleapis#2277, to see how much info
we can use to restrict the tests we run.
@dhermes dhermes force-pushed the dont-merge-2277-sleuthing branch from d4bc9d5 to 2a9b150 Compare November 15, 2016 17:16
@dhermes dhermes force-pushed the dont-merge-2277-sleuthing branch from dba3ef0 to d82a72b Compare November 15, 2016 19:40
@lukesneeringer
Copy link
Contributor

Superceded by #3146.

@dhermes dhermes deleted the dont-merge-2277-sleuthing branch March 16, 2017 23:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes This human has signed the Contributor License Agreement. do not merge Indicates a pull request not ready for merge, due to either quality or timing.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants