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

Intermittents #12629

Merged
merged 2 commits into from Aug 10, 2016
Merged

Intermittents #12629

merged 2 commits into from Aug 10, 2016

Conversation

@notriddle
Copy link
Contributor

notriddle commented Jul 28, 2016

We probably want to have Buildbot run that script.


  • ./mach build -d does not report any errors
  • ./mach test-tidy does not report any errors
  • These changes fix #12435, #11574, and #11561
  • These changes do not require tests because reasons

This change is Reviewable

@highfive
Copy link

highfive commented Jul 28, 2016

Heads up! This PR modifies the following files:

  • @aneeshusa: etc/ci/buildbot_steps.yml, etc/ci/check_intermittents.sh, etc/ci/former_intermittents_wpt.txt, etc/ci/former_intermittents_css.txt
@jdm
Copy link
Member

jdm commented Jul 28, 2016

@highfive highfive assigned aneeshusa and unassigned cbrewster Jul 28, 2016

for test_type in wpt css; do
while read test_name; do
echo " - Checking $test_name"

This comment has been minimized.

@wafflespeanut

wafflespeanut Jul 29, 2016

Member

Maybe a better message? like, "Checking $test_name because it's a known intermittent..."

This comment has been minimized.

@notriddle

notriddle Jul 29, 2016

Author Contributor

It's inside a script called check_intermittents, and every CI system in the world shows which command was run before its actual stdout. The only reason I included - Checking was to help distinguish it from the output of wptrunner itself.

@wafflespeanut
Copy link
Member

wafflespeanut commented Jul 29, 2016

This is a great idea! 👍

@@ -27,6 +27,14 @@ mac-nightly:
- ./mach package --release
- ./etc/ci/upload_nightly.sh mac

linux-intermittent:

This comment has been minimized.

@aneeshusa

aneeshusa Aug 4, 2016

Member

CI doesn't read from this file yet, it's currently reading from buildbot/master/files/config/steps.yml file in the saltfs repo, which is where we'll need to make this change.

Also, let's name these linux-rel-intermittent and mac-rel-intermittent for consistency.

@@ -0,0 +1,14 @@
#!/bin/sh

This comment has been minimized.

@aneeshusa

aneeshusa Aug 4, 2016

Member

Shebang should be #!/usr/bin/env bash per the style guide.

This comment has been minimized.

@aneeshusa

aneeshusa Aug 4, 2016

Member

Also, please add the errexit, nounset, and pipefail options.

This comment has been minimized.

@notriddle

notriddle Aug 4, 2016

Author Contributor

Done.

notriddle added a commit to notriddle/saltfs that referenced this pull request Aug 4, 2016
for test_type in wpt css; do
while read test_name; do
echo " - Checking $test_name"
./mach test-${test_type} --release --repeat $REPEAT_COUNT "$test_name" > tmp_log.txt

This comment has been minimized.

@aneeshusa

aneeshusa Aug 4, 2016

Member

Let's append to the log file always and have Buildbot handle showing the log (per this comment). Also, please use the full form and quotes "${some_variable}" for the variables (test_type and REPEAT_COUNT).

while read test_name; do
echo " - Checking $test_name"
./mach test-${test_type} --release --repeat $REPEAT_COUNT "$test_name" > tmp_log.txt
if [ $? != 0 ]; then

This comment has been minimized.

@aneeshusa

aneeshusa Aug 4, 2016

Member

Let set -o errexit handle the short circuit exiting.

cat tmp_log.txt
exit $?
fi
done < etc/ci/former_intermittents_${test_type}.txt

This comment has been minimized.

@aneeshusa

aneeshusa Aug 4, 2016

Member

Please quote the filename due to the variable substitution.

fi
done < etc/ci/former_intermittents_${test_type}.txt
done

This comment has been minimized.

@aneeshusa

aneeshusa Aug 4, 2016

Member

nit: remove the empty extra newline.

@@ -0,0 +1,13 @@
#!/usr/bin/env bash
REPEAT_COUNT=100

This comment has been minimized.

@aneeshusa

aneeshusa Aug 9, 2016

Member

Please put this after we set the bash options (e.g. errexit).

@aneeshusa
Copy link
Member

aneeshusa commented Aug 9, 2016

LGTM aside from the one nit, please also rebase on a new master (to pull in some new shell lints) and squash the fixups.

@notriddle
Copy link
Contributor Author

notriddle commented Aug 9, 2016

Rebased and moved.

@aneeshusa
Copy link
Member

aneeshusa commented Aug 9, 2016

Please fix the tidy failures.

@aneeshusa
Copy link
Member

aneeshusa commented Aug 10, 2016

@bors-servo
Copy link
Contributor

bors-servo commented Aug 10, 2016

📌 Commit c71cb19 has been approved by aneeshusa

@bors-servo
Copy link
Contributor

bors-servo commented Aug 10, 2016

Testing commit c71cb19 with merge 944cbbf...

bors-servo added a commit that referenced this pull request Aug 10, 2016
Intermittents

We probably want to have Buildbot run that script.

---

- [X] `./mach build -d` does not report any errors
- [X] `./mach test-tidy` does not report any errors
- [X] These changes fix #12435, #11574, and #11561
- [X] These changes do not require tests because reasons

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/12629)
<!-- Reviewable:end -->
@aneeshusa aneeshusa removed the S-fails-tidy label Aug 10, 2016
notriddle added a commit to notriddle/saltfs that referenced this pull request Aug 10, 2016
@bors-servo
Copy link
Contributor

bors-servo commented Aug 10, 2016

@bors-servo bors-servo merged commit c71cb19 into servo:master Aug 10, 2016
2 of 3 checks passed
2 of 3 checks passed
continuous-integration/appveyor/pr Waiting for AppVeyor build to complete
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
homu Test successful
Details
bors-servo added a commit to servo/saltfs that referenced this pull request Aug 10, 2016
Call intermittent checker

Part of servo/servo#12629

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/saltfs/453)
<!-- Reviewable:end -->
bors-servo added a commit to servo/saltfs that referenced this pull request Aug 17, 2016
Call intermittent checker

Part of servo/servo#12629

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/saltfs/453)
<!-- Reviewable:end -->
bors-servo added a commit to servo/saltfs that referenced this pull request Aug 18, 2016
Call intermittent checker

Part of servo/servo#12629

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/saltfs/453)
<!-- Reviewable:end -->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

7 participants
You can’t perform that action at this time.