Closed Bug 1237987 Opened 6 years ago Closed 6 years ago

Port changes made for audio over to tester image for mulet mochitests

Categories

(Taskcluster Graveyard :: Docker Images, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
2.6 S6 - 1/29

People

(Reporter: garndt, Assigned: garndt)

Details

Attachments

(1 file)

A lot of advancement has been made on the desktop-test image for video/audio related things that need to be ported over to the tester image and mulet mochitests should then be configured to use that image.

desktop-test is still being iterated upon for other test suites and should not be relied on for production (tier 1) jobs.
Assignee: nobody → garndt
Attachment #8706662 - Flags: review?(wcosta)
Comment on attachment 8706662 [details]
MozReview Request: Bug 1237987 - Update mulet mochitests to use tester image r=wcosta

https://reviewboard.mozilla.org/r/30427/#review27269

Just a few nit questions before r+ it.

::: testing/docker/tester/bin/test.sh:5
(Diff revision 1)
> +: GECKO_HEAD_REPOSITORY         ${GECKO_HEAD_REPOSITORY:=https://hg.mozilla.org/mozilla-central}

Whats the purpose of the colon at line beginning (just curious).

::: testing/taskcluster/scripts/tester/test-mulet.sh:26
(Diff revision 1)
> +if [[ -z ${MOZHARNESS_URL} ]]; then exit 1; fi

Can't we just `test ${MOZNARNESS_URL}` or, at worse, `test ${MOZHANRESS_URL} || exit 1`?

::: testing/taskcluster/scripts/tester/test-mulet.sh:35
(Diff revision 1)
> +    if [ -n "$xvfb_pid" ] && [ $START_VNC == false ] ; then

make `$START_VNC` surrounded by double quotes so the expression is still valid if the variable isn't defined.

::: testing/taskcluster/scripts/tester/test-mulet.sh:74
(Diff revision 1)
> +        if [ $xvfb_test != 255 ]; then

Shouldn't we break the loop if `$xvfb_test` is different from 255?
Thanks for the review and I'll take a look at the comments.  test-mulet.sh was a copy of the already existing test-linux.sh that's being used for desktop tests.
I would like to port any improvements to the script to test-linux.sh where possible. Thanks!
If you could port them that would be great, otherwise, I will look at what lands and compare.
> (Diff revision 1)
> > +: GECKO_HEAD_REPOSITORY         ${GECKO_HEAD_REPOSITORY:=https://hg.mozilla.org/mozilla-central}

I've seen it commonly used with assigning default values to things. I think it's the same as if one did:
GECKO_HEAD_REPOSITORY=${GECKO_HEAD_REPOSITORY:-https://hg.mozilla.org/mozilla-central}
https://reviewboard.mozilla.org/r/30427/#review27595

::: testing/taskcluster/scripts/tester/test-mulet.sh:74
(Diff revision 1)
> +        if [ $xvfb_test != 255 ]; then

Ok, I'll drop this issue this because the logic seems to be correct.
Comment on attachment 8706662 [details]
MozReview Request: Bug 1237987 - Update mulet mochitests to use tester image r=wcosta

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/30427/diff/1-2/
Attachment #8706662 - Flags: review?(wcosta)
Comment on attachment 8706662 [details]
MozReview Request: Bug 1237987 - Update mulet mochitests to use tester image r=wcosta

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/30427/diff/2-3/
Comment on attachment 8706662 [details]
MozReview Request: Bug 1237987 - Update mulet mochitests to use tester image r=wcosta

https://reviewboard.mozilla.org/r/30427/#review27761
Attachment #8706662 - Flags: review?(wcosta) → review+
https://hg.mozilla.org/mozilla-central/rev/7c56c933b75b
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → 2.6 S6 - 1/29
Product: Taskcluster → Taskcluster Graveyard
You need to log in before you can comment on or make changes to this bug.