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

Stability checker running totally unrelated tests #5408

Closed
gsnedders opened this issue Apr 6, 2017 · 4 comments
Closed

Stability checker running totally unrelated tests #5408

gsnedders opened this issue Apr 6, 2017 · 4 comments
Assignees

Comments

@gsnedders
Copy link
Member

see https://travis-ci.org/w3c/web-platform-tests/builds/218610626, which happened to hit some tests that Chrome has pre-existing instability on (the same set of tests is run in both Firefox and Chrome)

cc/ @jgraham @jugglinmike @bobholt

@jugglinmike
Copy link
Contributor

Self-assigning this one.

@gsnedders
Copy link
Member Author

Potentially has overlap with #5406.

@jugglinmike
Copy link
Contributor

For builds triggered by a pull request, the "check stability" script uses an environment variable provided by TravisCI named TRAVIS_COMMIT_RANGE:

def get_branch_point(user):
    git = get_git_cmd(wpt_root)
    if os.environ.get("TRAVIS_PULL_REQUEST", "false") != "false":
        # This is a PR, so the base branch is in TRAVIS_BRANCH
        branch_point = os.environ.get("TRAVIS_COMMIT_RANGE").split(".", 1)[0]
branch_point = git("rev-parse", branch_point)

Here is the relevant documentation

  • TRAVIS_COMMIT_RANGE: The range of commits that were included in the push
    or pull request. (Note that this is empty for builds triggered by the
    initial commit of a new branch.)

That description is somewhat imprecise given that there are a number of ways to specify a range using git's revision syntax. More troubling than the ambiguity, though, is that the value seems to be simply incorrect in some cases. In this demonstration, the "left-hand side" of the range is a SHA that references a seemingly-arbitrary commit on the master branch. I've reported this to the TravisCI project but have yet to hear back.

My working theory is that this situation occurs whenever commits are merged to the target branch after the pull request has been opened (or possibly just after the job has started to run). In any event, the variable is clearly not stable enough for our purposes here.

My plan is to instead use the TRAVIS_BRANCH environment variable, which is documented as follows:

  • TRAVIS_BRANCH:
    • for push builds, or builds not triggered by a pull request, this is the
      name of the branch.
    • for builds triggered by a pull request this is the name of the branch
      targeted by the pull request.

Not only is this definition much less ambiguous, it is also less susceptible to the kind of race conditions I described above (especially considering that the script fetches from the master branch immediately prior to this step.

@jugglinmike
Copy link
Contributor

Resolved via gh-5416.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants