Chris Manchester (limited bugmail, email directly)

Comment 3

•

10 years ago

This error makes me think that you have a copy of firefox that doesn't have a marionette server recent enough. From that log 368c62292249 is from before the relevant command landed. We've since uplifted everything to aurora, but let's try first with a more recent nightly (something from earlier this week should do). We also need marionette packages that were released only earlier this week, but from the log there I think you already have them.

Armen [:armenzg]

Assignee

Comment 4

•

10 years ago

Thanks chmanchester. This is my output for the run against a nightly from two days ago (3-29) [1]. I will run it against last night's to see how it differs. I noticed a long wait after an update has downloaded. [1] ftp://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/2015-03-29-03-02-38-mozilla-central/firefox-39.0a1.en-US.mac.dmg (venv)MacAir firefox-ui-tests git:[master] $ firefox-ui-update --binary temp/firefox/FirefoxNightly.app/Contents/MacOS/firefox 0:00.00 LOG: MainThread INFO Creating a copy of the application at "/var/folders/wl/l70t_q8n6gg64vr7bfzjrqg80000gn/T/tmp0G8aHG.binary-update-tests". 0:07.52 LOG: MainThread INFO starting httpd 0:07.52 LOG: MainThread INFO running webserver on http://127.0.0.1:55068/ 0:07.53 LOG: MainThread mozversion INFO application_buildid: 20150329030238 0:07.53 LOG: MainThread mozversion INFO application_changeset: 385840329d91 0:07.53 LOG: MainThread mozversion INFO application_display_name: Nightly 0:07.53 LOG: MainThread mozversion INFO application_id: {ec8030f7-c20a-464f-9b0e-13a3a9e97384} 0:07.53 LOG: MainThread mozversion INFO application_name: Firefox 0:07.53 LOG: MainThread mozversion INFO application_remotingname: firefox 0:07.53 LOG: MainThread mozversion INFO application_repository: https://hg.mozilla.org/mozilla-central 0:07.53 LOG: MainThread mozversion INFO application_vendor: Mozilla 0:07.53 LOG: MainThread mozversion INFO application_version: 39.0a1 0:07.53 LOG: MainThread mozversion INFO platform_buildid: 20150329030238 0:07.53 LOG: MainThread mozversion INFO platform_changeset: 385840329d91 0:07.53 LOG: MainThread mozversion INFO platform_repository: https://hg.mozilla.org/mozilla-central 0:07.53 LOG: MainThread mozversion INFO platform_version: 39.0a1 0:07.53 SUITE_START: MainThread 1 0:07.53 TEST_START: MainThread test_direct_update.py TestDirectUpdate.test_update 1:00.11 LOG: MainThread INFO Update test results: [{'patch': {'url_mirror': u'http://download.cdn.mozilla.net/pub/mozilla.org/firefox/nightly/2015/03/2015-03-31-03-02-04-mozilla-central/firefox-40.0a1.en-US.mac.complete.mar', 'buildid': u'20150331030204', 'version': u'40.0a1', 'download_duration': 21.309561, 'type': u'minor', 'is_complete': True, 'channel': u'nightly', 'size': 96671646}, 'fallback': False, 'build_pre': {'disabled_addons': None, 'mar_channels': set([u'firefox-mozilla-central']), 'user_agent': u'Mozilla/5.0 (Macintosh; Intel Mac OS X 10.10; rv:39.0) Gecko/20100101 Firefox/39.0', 'version': u'39.0a1', 'buildid': u'20150329030238', 'locale': u'en-US', 'url_aus': u'https://aus4.mozilla.org/update/3/Firefox/39.0a1/20150329030238/Darwin_x86_64-gcc3-u-i386-x86_64/en-US/nightly/Darwin%2014.1.0/default/default/update.xml?force=1', 'channel': u'nightly'}, 'success': True, 'build_post': {'disabled_addons': None, 'mar_channels': set([u'firefox-mozilla-central']), 'user_agent': u'Mozilla/5.0 (Macintosh; Intel Mac OS X 10.10; rv:40.0) Gecko/20100101 Firefox/40.0', 'version': u'40.0a1', 'buildid': u'20150331030204', 'locale': u'en-US', 'url_aus': u'https://aus4.mozilla.org/update/3/Firefox/40.0a1/20150331030204/Darwin_x86_64-gcc3-u-i386-x86_64/en-US/nightly/Darwin%2014.1.0/default/default/update.xml?force=1', 'channel': u'nightly'}}] 1:00.27 TEST_END: MainThread ERROR, expected PASS Traceback (most recent call last): File "/Users/armenzg/repos/firefox-ui-tests/temp/venv/lib/python2.7/site-packages/marionette_client-0.9.2-py2.7.egg/marionette/marionette_test.py", line 326, in run self.tearDown() File "/Users/armenzg/repos/firefox-ui-tests/firefox_ui_tests/update/direct/test_direct_update.py", line 17, in tearDown UpdateTestCase.tearDown(self) File "/Users/armenzg/repos/firefox-ui-tests/firefox_ui_harness/testcases/update.py", line 232, in tearDown FirefoxTestCase.tearDown(self) File "/Users/armenzg/repos/firefox-ui-tests/firefox_ui_harness/testcase.py", line 67, in tearDown (self._start_handle_count, win_count)) AssertionError: A test must not leak window handles. This test started the browser with 1 open top level browsing contexts, but ended with 2. 1:00.27 LOG: MainThread INFO SUMMARY ------- 1:00.27 LOG: MainThread INFO passed: 0 1:00.27 LOG: MainThread INFO failed: 1 1:00.27 LOG: MainThread INFO todo: 0 1:00.27 LOG: MainThread INFO FAILED TESTS ------- 1:00.28 LOG: MainThread INFO test_direct_update.py test_direct_update.TestDirectUpdate.test_update 1:00.34 SUITE_END: MainThread Summary ======= Ran 1 tests Expected results: 0 Unexpected results: 1 (ERROR: 1) Unexpected Results ================== ERROR test_direct_update.py TestDirectUpdate.test_update 1:00.34 LOG: MainThread INFO Removing copy of the application at "/var/folders/wl/l70t_q8n6gg64vr7bfzjrqg80000gn/T/tmp0G8aHG.binary-update-tests" 1:00.39 LOG: MainThread INFO Creating a copy of the application at "/var/folders/wl/l70t_q8n6gg64vr7bfzjrqg80000gn/T/tmpxBcdER.binary-update-tests". ^@ 1:04.25 LOG: MainThread mozversion INFO application_buildid: 20150329030238 1:04.25 LOG: MainThread mozversion INFO application_changeset: 385840329d91 1:04.25 LOG: MainThread mozversion INFO application_display_name: Nightly 1:04.25 LOG: MainThread mozversion INFO application_id: {ec8030f7-c20a-464f-9b0e-13a3a9e97384} 1:04.25 LOG: MainThread mozversion INFO application_name: Firefox 1:04.25 LOG: MainThread mozversion INFO application_remotingname: firefox 1:04.25 LOG: MainThread mozversion INFO application_repository: https://hg.mozilla.org/mozilla-central 1:04.25 LOG: MainThread mozversion INFO application_vendor: Mozilla 1:04.25 LOG: MainThread mozversion INFO application_version: 39.0a1 1:04.25 LOG: MainThread mozversion INFO platform_buildid: 20150329030238 1:04.25 LOG: MainThread mozversion INFO platform_changeset: 385840329d91 1:04.25 LOG: MainThread mozversion INFO platform_repository: https://hg.mozilla.org/mozilla-central 1:04.25 LOG: MainThread mozversion INFO platform_version: 39.0a1 1:04.25 SUITE_START: MainThread 1 1:04.25 TEST_START: MainThread test_fallback_update.py TestFallbackUpdate.test_update ^@ 2:33.86 LOG: MainThread INFO Update test results: [{'patch': {'url_mirror': u'http://download.cdn.mozilla.net/pub/mozilla.org/firefox/nightly/2015/03/2015-03-31-03-02-04-mozilla-central/firefox-40.0a1.en-US.mac.complete.mar', 'buildid': u'20150331030204', 'version': u'40.0a1', 'download_duration': 16.425093, 'type': u'minor', 'is_complete': True, 'channel': u'nightly', 'size': 96671646}, 'fallback': True, 'build_pre': {'disabled_addons': None, 'mar_channels': set([u'firefox-mozilla-central']), 'user_agent': u'Mozilla/5.0 (Macintosh; Intel Mac OS X 10.10; rv:39.0) Gecko/20100101 Firefox/39.0', 'version': u'39.0a1', 'buildid': u'20150329030238', 'locale': u'en-US', 'url_aus': u'https://aus4.mozilla.org/update/3/Firefox/39.0a1/20150329030238/Darwin_x86_64-gcc3-u-i386-x86_64/en-US/nightly/Darwin%2014.1.0/default/default/update.xml?force=1', 'channel': u'nightly'}, 'success': True, 'build_post': {'disabled_addons': None, 'mar_channels': set([u'firefox-mozilla-central']), 'user_agent': u'Mozilla/5.0 (Macintosh; Intel Mac OS X 10.10; rv:40.0) Gecko/20100101 Firefox/40.0', 'version': u'40.0a1', 'buildid': u'20150331030204', 'locale': u'en-US', 'url_aus': u'https://aus4.mozilla.org/update/3/Firefox/40.0a1/20150331030204/Darwin_x86_64-gcc3-u-i386-x86_64/en-US/nightly/Darwin%2014.1.0/default/default/update.xml?force=1', 'channel': u'nightly'}}] 2:33.99 TEST_END: MainThread ERROR, expected PASS Traceback (most recent call last): File "/Users/armenzg/repos/firefox-ui-tests/temp/venv/lib/python2.7/site-packages/marionette_client-0.9.2-py2.7.egg/marionette/marionette_test.py", line 326, in run self.tearDown() File "/Users/armenzg/repos/firefox-ui-tests/firefox_ui_tests/update/fallback/test_fallback_update.py", line 17, in tearDown UpdateTestCase.tearDown(self) File "/Users/armenzg/repos/firefox-ui-tests/firefox_ui_harness/testcases/update.py", line 232, in tearDown FirefoxTestCase.tearDown(self) File "/Users/armenzg/repos/firefox-ui-tests/firefox_ui_harness/testcase.py", line 67, in tearDown (self._start_handle_count, win_count)) AssertionError: A test must not leak window handles. This test started the browser with 1 open top level browsing contexts, but ended with 2. 2:34.00 LOG: MainThread INFO SUMMARY ------- 2:34.00 LOG: MainThread INFO passed: 0 2:34.00 LOG: MainThread INFO failed: 1 2:34.00 LOG: MainThread INFO todo: 0 2:34.00 LOG: MainThread INFO FAILED TESTS ------- 2:34.00 LOG: MainThread INFO test_fallback_update.py test_fallback_update.TestFallbackUpdate.test_update 2:34.07 SUITE_END: MainThread Summary ======= Ran 1 tests Expected results: 0 Unexpected results: 1 (ERROR: 1) Unexpected Results ================== ERROR test_fallback_update.py TestFallbackUpdate.test_update 2:34.07 LOG: MainThread INFO Removing copy of the application at "/var/folders/wl/l70t_q8n6gg64vr7bfzjrqg80000gn/T/tmpxBcdER.binary-update-tests"

Armen [:armenzg]

Assignee

Comment 5

•

10 years ago

Attached file run.txt (obsolete) — Details

My current worry is trying to understand the very very long wait we get into. Attached is the run with the failed tests. I will try adding a timeout to fail sooner. wget ftp://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/2015-03-30-11-48-16-mozilla-central/firefox-40.0a1.en-US.mac.dmg mozinstall --app=firefox firefox-40.0a1.en-US.mac.dmg firefox-ui-update --binary FirefoxNightly.app/Contents/MacOS/firefox --update-direct-only --gecko-log -

Armen [:armenzg]

Assignee

Comment 6

•

10 years ago

I can't reproduce the issue anymore.

Armen [:armenzg]

Assignee

Comment 7

•

10 years ago

Why do we start a web server? http://127.0.0.1:58569

Chris Manchester (limited bugmail, email directly)

Comment 8

•

10 years ago

(In reply to Armen Zambrano G. (:armenzg - Toronto) from comment #7) > Why do we start a web server? > http://127.0.0.1:58569 Marionette's test harness always starts a local webserver to serve files used by tests. We could probably make it opt-out (and then opt-out in update tests) if it's going to cause problems.

Armen [:armenzg]

Assignee

Comment 9

•

10 years ago

Just curious. We probably won't need to worry. Thanks!

Chris Manchester (limited bugmail, email directly)

Comment 10

•

10 years ago

•

•

10 years ago

Attached file errors.txt (obsolete) — Details

Hi Chris, What are your suggestion to recuperate from this error? (See attachment) I think the httpd server got into a weird state.

Attachment #8589292 - Flags: feedback?(cmanchester)

Armen [:armenzg]

Assignee

Comment 17

•

10 years ago

This is the latest comparison (includes changes from today): https://github.com/mozilla/build-tools/compare/master...armenzg:update_testing I managed to make progress on the loaner, however, it seems that it broke the connection and the run stopped. I started a screen session and I'm trying again with: time ./verify.sh --marionette mozBeta-firefox-linux64.cfg | tee fullrun.txt

Chris Manchester (limited bugmail, email directly)

Comment 18

•

10 years ago

Comment on attachment 8589292 [details] errors.txt This is pretty messy, but I think it all starts with an actual failure: 6:14.77 TEST_END: MainThread FAIL, expected PASS Traceback (most recent call last): File "/home/cltbld/temp/build-tools/release/updates/venv/lib/python2.7/site-packages/marionette/marionette_test.py", line 296, in run testMethod() File "/home/cltbld/temp/build-tools/release/updates/venv/lib/python2.7/site-packages/firefox_ui_tests/update/direct/test_direct_update.py", line 20, in test_update self.download_and_apply_available_update(force_fallback=False) File "/home/cltbld/temp/build-tools/release/updates/venv/lib/python2.7/site-packages/firefox_ui_harness/testcases/update.py", line 149, in download_and_apply_available_update self.assertTrue(update_available) AssertionError: False is not true ...then marionette times out trying to take a screenshot, and dies in a way that causes the address unavailable error when the next set of tests run. I think the whole thing would be easier to understand if we didn't run the second set of tests if the first failed, or if we waited 30s for the socket to be usable again. Is your main question about the cause of the failure, or how to get better diagnostic information? I'll download this build now and see if I can reproduce the failure locally.

Flags: needinfo?(armenzg)

Attachment #8589292 - Flags: feedback?(cmanchester)

Chris Manchester (limited bugmail, email directly)

Comment 19

•

10 years ago

When I try the tests locally with the build referenced in the log it hangs a similar point to that original failure. There's no text or UI or text rendered in the about window when the test opens it (the same is true opening the about window independent of the tests). If I resize the about window the normal text appears.

Armen [:armenzg]

Assignee

Comment 20

•

10 years ago

•

10 years ago

I may have misremembered that timeout duration, it might be much longer. Would trying a different port help here?

Flags: needinfo?(cmanchester)

Armen [:armenzg]

Assignee

Comment 27

•

10 years ago

(In reply to Chris Manchester [:chmanchester] from comment #26) > I may have misremembered that timeout duration, it might be much longer. > Would trying a different port help here? I believe so.

Armen [:armenzg]

Assignee

•

10 years ago

Attached patch add marionette as an option to verify.sh (obsolete) — Details — Splinter Review

Flags: needinfo?(armenzg)

Attachment #8591010 - Flags: feedback?(bhearsum)

Armen [:armenzg]

Assignee

Comment 32

•

10 years ago

(In reply to Ben Hearsum [:bhearsum] from comment #30) > (In reply to Armen Zambrano G. (:armenzg - Toronto) from comment #29) > > This comes from executing this code: > > https://github.com/mozilla/build-tools/compare/master...armenzg: > > update_testing > > > > Could you please have a look at it and give me feedback? > > I can attach a normal patch if prefered. > > Please do. Is this log just to aid in my feedback on the code, or are you > looking for feedback on some part of the output as well? > To help with the feedback on the code (which I have just attached). You can see in the output both good runs and bad runs. The gecko.log output could be useful when debugging issues. > > On another note, could you please help me by adding some builders that will > > execute code from my user tools repo? > > I don't have time to do this right now. I can give you some guidance if you > want to give it a shot though. > When do you think you could have time? If not in the next couple of weeks, I would be happy to at least have a bit of guidance. I haven't touched (sepcifically) release config paths in a very long time. > > That way I can run things in the real environments rather than through a > > screen session. > > For some reason, the output of my run under screen does not show completely > > in this output. > > If I had to guess, I'd say you're probably only capturing stdout and not > stderr. What I meant to say is that it suddenly stopped. When I connected to the screen there was still some output that was not in fullrun.txt You can also tell by looking at the attachment and see that the output stopped mid-sentence. I took stderr redirection into account in the command I run. For reference: time ./verify.sh --marionette mozBeta-firefox-linux64.cfg 2>&1 | tee fullrun.txt

bhearsum@mozilla.com (:bhearsum)

Comment 33

•

10 years ago

Comment on attachment 8591010 [details] [diff] [review] add marionette as an option to verify.sh Review of attachment 8591010 [details] [diff] [review]: ----------------------------------------------------------------- This is a good start, most of the issues below are about style/consistency, and making sure things work on all platforms. ::: release/updates/verify.sh @@ +50,5 @@ > + url="${ftp_server_from}/${from_path}" > + source_file=`basename "$url"` > + if [ -f "$source_file" ]; then rm "$source_file"; fi > + cached_download "$source_file" "${ftp_server_from}/${from_path}" > +} Why do you need this function? We already have download_builds in https://github.com/mozilla/build-tools/blob/master/release/common/download_builds.sh... @@ +52,5 @@ > + if [ -f "$source_file" ]; then rm "$source_file"; fi > + cached_download "$source_file" "${ftp_server_from}/${from_path}" > +} > + > +run_marionette_tests() Minor nit, but it would be more consistent with the rest of this script if you put the body of this method in block down below rather than calling out to this. @@ +53,5 @@ > + cached_download "$source_file" "${ftp_server_from}/${from_path}" > +} > + > +run_marionette_tests() > +{ Most of this isn't going to work on Windows, and some of it not on Mac. Some comments inline: @@ +55,5 @@ > + > +run_marionette_tests() > +{ > + echo "Running Marionnete tests." > + export DISPLAY=:2 You shouldn't need to set this on actual build/test machines. Buildbot (or something above it) will set it where necessary. @@ +70,5 @@ > + then > + netstat -atnp | grep 2828 > + echo "Killing Firefox (if any) to ensure clean state." > + killall -9 firefox > + fi It's going to be next to impossible to do process management in a cross platform way in bash. I strongly recommend looking at moving this into the Marionette harness. IIRC we have code in other test harnesses that does similar things already. @@ +75,5 @@ > + # We should optimize this; unpack_build inside of check_updates already unpacks this once > + rm -rf $release > + mkdir $release > + echo "Unpacking $source_file..." > + mozinstall -d $release $source_file I'd rather see you use the same unpacking mechanisms that the rest of update verify uses. Specifically: https://github.com/mozilla/build-tools/blob/master/release/common/unpack.sh. It's going to be very confusing if we start having update verify runs that fail to unpack builds only for certain test types. @@ +89,5 @@ > + else > + echo "== Dumping good run output ==" > + cat short_log.txt > + echo "== End of good run outputt ==" > + fi It's nice to see an update verify test that doesn't spam the logs so much on success, good job! @@ +123,5 @@ > + runmode=$MARIONETTE > + shift > + ;; > + --dont-clear-cache) > + dontclear=1 I think it's fine to have this option, but I don't see any us ever wanting to use it in production. We run these tests in very large batches, and we usually want to start with a clear cache at the start. @@ +167,5 @@ > + venv="$(pwd)/venv" > + if [ -z $dontclear ] > + then > + rm -rf $venv > + fi I don't think it's good to conflate dontclear with the cache as well as virtualenv. I think it's fine to always delete the virtualenv.

Attachment #8591010 - Flags: feedback?(bhearsum) → feedback+

bhearsum@mozilla.com (:bhearsum)

•

10 years ago

> @@ +167,5 @@ > > + venv="$(pwd)/venv" > > + if [ -z $dontclear ] > > + then > > + rm -rf $venv > > + fi > > I don't think it's good to conflate dontclear with the cache as well as > virtualenv. I think it's fine to always delete the virtualenv. It helps me to speed up the testing.

bhearsum@mozilla.com (:bhearsum)

Comment 37

•

10 years ago

(In reply to Armen Zambrano G. (:armenzg - Toronto) from comment #36) > > @@ +167,5 @@ > > > + venv="$(pwd)/venv" > > > + if [ -z $dontclear ] > > > + then > > > + rm -rf $venv > > > + fi > > > > I don't think it's good to conflate dontclear with the cache as well as > > virtualenv. I think it's fine to always delete the virtualenv. > > It helps me to speed up the testing. That's certainly a valid reason, but I don't think it's a good argument for controlling it with --dont-clear-cache. You've got a couple of options: * Use a separate flag for controlling virtualenv cleanup * Clean up the virtualenv yourself before running the script (In reply to Armen Zambrano G. (:armenzg - Toronto) from comment #35) > (In reply to Ben Hearsum [:bhearsum] from comment #33) > > ::: release/updates/verify.sh > > @@ +50,5 @@ > > > + url="${ftp_server_from}/${from_path}" > > > + source_file=`basename "$url"` > > > + if [ -f "$source_file" ]; then rm "$source_file"; fi > > > + cached_download "$source_file" "${ftp_server_from}/${from_path}" > > > +} > > > > Why do you need this function? We already have download_builds in > > https://github.com/mozilla/build-tools/blob/master/release/common/ > > download_builds.sh... > > It expects me to download 2 binaries, not one: > https://github.com/mozilla/build-tools/blob/master/release/common/ > download_builds.sh#L14 OK, two options then: * Tweak download_builds to work with either 1 or 2 URLs * Change it only take one URL, and adjust any existing calls to make two separate calls.

Armen [:armenzg]

Assignee

Updated

•

10 years ago

Depends on: 1154047

Armen [:armenzg]

Assignee

Comment 38

•

10 years ago

Attached file fullrun.txt (obsolete) — Details

I think I have addressed all of your comments.

Attachment #8586150 - Attachment is obsolete: true

Attachment #8587599 - Attachment is obsolete: true

Attachment #8589292 - Attachment is obsolete: true

Attachment #8589898 - Attachment is obsolete: true

Attachment #8590919 - Attachment is obsolete: true

Attachment #8591010 - Attachment is obsolete: true

Attachment #8591933 - Flags: feedback?(bhearsum)

Armen [:armenzg]

Assignee

Comment 39

•

10 years ago

(In reply to Armen Zambrano G. (:armenzg - Toronto) from comment #32) ... > > > That way I can run things in the real environments rather than through a > > > screen session. > > > For some reason, the output of my run under screen does not show completely > > > in this output. > > > > If I had to guess, I'd say you're probably only capturing stdout and not > > stderr. > > What I meant to say is that it suddenly stopped. > When I connected to the screen there was still some output that was not in > fullrun.txt > You can also tell by looking at the attachment and see that the output > stopped mid-sentence. > > I took stderr redirection into account in the command I run. For reference: > time ./verify.sh --marionette mozBeta-firefox-linux64.cfg 2>&1 | tee > fullrun.txt It seems that I was running out of disk space (bug 1154060). bhearsum: Since we don't reboot build machines all the time; should I also add something like this: rm -rf /tmp/*binary-update-tests?

Armen [:armenzg]

Assignee

Updated

•

10 years ago

No longer depends on: 1149628, 1150620

Comment 40

•

10 years ago

Bug 1150679 and bug 1152460 are not real blockers. They *might* be nice to have.

No longer depends on: 1150679, 1152460

Comment 41

•

10 years ago

(In reply to Armen Zambrano G. (:armenzg - Toronto) from comment #39) > (In reply to Armen Zambrano G. (:armenzg - Toronto) from comment #32) > ... > > > > That way I can run things in the real environments rather than through a > > > > screen session. > > > > For some reason, the output of my run under screen does not show completely > > > > in this output. > > > > > > If I had to guess, I'd say you're probably only capturing stdout and not > > > stderr. > > > > What I meant to say is that it suddenly stopped. > > When I connected to the screen there was still some output that was not in > > fullrun.txt > > You can also tell by looking at the attachment and see that the output > > stopped mid-sentence. > > > > I took stderr redirection into account in the command I run. For reference: > > time ./verify.sh --marionette mozBeta-firefox-linux64.cfg 2>&1 | tee > > fullrun.txt > > It seems that I was running out of disk space (bug 1154060). > > > bhearsum: Since we don't reboot build machines all the time; should I also > add something like this: > rm -rf /tmp/*binary-update-tests? You shouldn't need to manage this from within update verify. Runner deals with machine cleanup these days. You may need to do something by hand when testing locally. Is there supposed to be another patch attached to this bug? I only see a log...

Flags: needinfo?(armenzg)

Armen [:armenzg]

Assignee

Comment 42

•

10 years ago

Attached patch tools.diff (obsolete) — Details — Splinter Review

Here's the patch.

Flags: needinfo?(armenzg)

Attachment #8592214 - Flags: feedback?(bhearsum)

bhearsum@mozilla.com (:bhearsum)

Comment 43

•

10 years ago

Comment on attachment 8592214 [details] [diff] [review] tools.diff Review of attachment 8592214 [details] [diff] [review]: ----------------------------------------------------------------- Definite feedback+ on this. With the one comment below addressed I think this could land, as long as you've made sure that this won't break existing update verify jobs. ::: release/updates/verify.sh @@ +162,5 @@ > + netstat -atnp | grep 2828 > + echo "We have a socket open on 2828 without a pid. We need to sleep few times." > + sleep 10 > + done > + fi Have you tested this on Windows, Mac, and Linux? I see that Windows _does_ have netstat, but I would be mildly surprised if all three platforms work the same way. I strongly urge you to move this into firefox-ui-update or Marionette itself - I anticipate that dealing with edge cases here is going to be very painful. Perhaps a different approach here would be better. Why is it that firefox-ui-update will leave things lingering? Can you stop _that_ from happening in the first place instead of trying to deal with the fallout? That would speed up testing too, because we wouldn't be sleeping while waiting for something to go away.

Attachment #8592214 - Flags: feedback?(bhearsum) → feedback+

bhearsum@mozilla.com (:bhearsum)

Updated

•

10 years ago

Attachment #8591933 - Flags: feedback?(bhearsum)

Armen [:armenzg]

Assignee

Comment 44

•

10 years ago

chmanchester: what do you suggest we do about what Ben mentions below? [1] We have spoken about using other ports in the past. We could move the sleeping to firefox-ui-updates. And/or find the root issue. This is one comment we had about the topic: https://bugzilla.mozilla.org/show_bug.cgi?id=1152444#c1 > Another thing I have noticed is that if I kill the harness we have a socket held for 30 seconds with TIME_WAIT > (venv)armenzg@armenzg-thinkpad:~/repos/firefox-ui-tests$ netstat -anp | grep ':2828 ' | grep TIME_WAIT > (Not all processes could be identified, non-owned process info > will not be shown, you would have to be root to see it all.) > tcp 0 0 127.0.0.1:2828 127.0.0.1:47185 TIME_WAIT - > > I believe we have two cases where the port might be already in use. > * One of them is when firefox gets into a bad state and holds a socket with LISTEN > * The other one is when "the Python client shuts down the socket abruptly (due to a global timeout on the operation, > for instance)" [1] What bherasum said: > ::: release/updates/verify.sh > @@ +162,5 @@ > > + netstat -atnp | grep 2828 > > + echo "We have a socket open on 2828 without a pid. We need to sleep few times." > > + sleep 10 > > + done > > + fi > > > Perhaps a different approach here would be better. Why is it that > firefox-ui-update will leave things lingering? Can you stop _that_ from > happening in the first place instead of trying to deal with the fallout? > That would speed up testing too, because we wouldn't be sleeping while > waiting for something to go away. Addressed to me: > Have you tested this on Windows, Mac, and Linux? I see that Windows _does_ > have netstat, but I would be mildly surprised if all three platforms work > the same way. I strongly urge you to move this into firefox-ui-update or > Marionette itself - I anticipate that dealing with edge cases here is going > to be very painful. I was hoping to test on Windows and Mac in the infrastructure (or loan machines) but for a handful of locales I can do it locally w/o hurting my bandwidth limit :)

Flags: needinfo?(cmanchester)

Armen [:armenzg]

Assignee

Comment 45

•

Armen [:armenzg]

Assignee

Comment 57

•

10 years ago

My apologies for not making it more clear. I will need to grab a Windows and Mac loaners to make dry runs once all the various remaining issues are addressed. I will be adding a local development flag to make everything more clear. My apologies if I was not more explicit about this. (In reply to Ben Hearsum [:bhearsum] from comment #56) > Comment on attachment 8594969 [details] [diff] [review] > [tools] Add marionette update testing mode > > Review of attachment 8594969 [details] [diff] [review]: > ----------------------------------------------------------------- > > ::: release/common/setup_marionette.sh > @@ +2,5 @@ > > +venv_dir="$1" > > + > > +if [ ! -f $HOME/.pip/pip.conf ] > > +then > > + pip_options="--no-index --find-links http://pypi.pub.build.mozilla.org/pub --trusted-host pypi.pub.build.mozilla.org" > > You shouldn't hard code hosts like this into the script This is mainly for local machines. I will add a flag for local development. > @@ +12,5 @@ > > + if [ "`uname -o`" == "Msys" ] > > + then > > + pip_path=$venv_dir/Scripts/pip > > + # win32api is needed by retry.py > > + $venv_dir/Scripts/easy_install http://pypi.pub.build.mozilla.org/pub/pywin32-216.win32-py2.7.exe > > I'm a bit surprised you need this considering our Buildbot install requires > it. I guess our Buildbot install is using a different Python installation... > > In any case, is there a reason you're not pip installing it like the other > packages? We need to find a way of installing this that doesn't have you > hardcoding a hostname into the update verify scripts. > Local development. I will fix it. pywin32 is installed with easy_install even on mozharness. I could not use --no-site-packages and use what is in the releng host. I would still need to fix this for local development. > ::: release/updates/verify.sh > @@ +125,5 @@ > > + then > > + export PATH=$venv/Scripts:$PATH > > + else > > + export PATH=$venv/bin:$PATH > > + fi > > I know this path exporting isn't totally new in this patch, but it's > surprising to me. Why do you need to mess with PATH when you've already > activated a virtualenv? Something smells fishy here. > I have not activate the virtualenv in any spot. I will switch to activating it. > @@ +161,5 @@ > > + then > > + echo "Running Marionnete tests for $product $release $locale" > > + # cleanup > > + mkdir -p downloads/ > > + rm -rf downloads/* > > Can you explain why you've taken this out of download_build? Given that it > is called a few lines down, I don't understand why you've pulled it out only > to call it here (and further down). > We're switching from this: > download_builds "${ftp_server_from}/${from_path}" "${ftp_server_to}/${to_path}" to this: > download_build "${ftp_server_from}/${from_path}" > download_build "${ftp_server_to}/${to_path}" If I leave the removal inside of download_build, the first download is as it would have never happened. I can add a do_not_clobber parameter and set to True on the first call if you prefer to. > @@ +167,5 @@ > > + # Make sure there is nothing lingering left from previous runs. > > + # We're hoping to improve this in firefox-ui-tests > > + # Bug https://bugzilla.mozilla.org/show_bug.cgi?id=1152460 > > + if [ `netstat -anp 2>/dev/null | grep ':2828.*127' | grep TIME_WAIT | wc -l` -ne 0 ] > > + then > > Has this been tested on all platforms yet? I can't r+ unless you know it > works.... This will be removed in the final product. It's worked though locally for me on Windows and Mac.

bhearsum@mozilla.com (:bhearsum)

Comment 58

•

10 years ago

(In reply to Armen Zambrano G. (:armenzg - Toronto) from comment #57) > My apologies for not making it more clear. > I will need to grab a Windows and Mac loaners to make dry runs once all the > various remaining issues are addressed. > I will be adding a local development flag to make everything more clear. > My apologies if I was not more explicit about this. Given that all of this local development stuff is about setting up the environment correctly, I'd much rather see a wrapper script around update verify than this logic embedded deep inside it. What do you think about that? > (In reply to Ben Hearsum [:bhearsum] from comment #56) > > @@ +161,5 @@ > > > + then > > > + echo "Running Marionnete tests for $product $release $locale" > > > + # cleanup > > > + mkdir -p downloads/ > > > + rm -rf downloads/* > > > > Can you explain why you've taken this out of download_build? Given that it > > is called a few lines down, I don't understand why you've pulled it out only > > to call it here (and further down). > > > We're switching from this: > > download_builds "${ftp_server_from}/${from_path}" "${ftp_server_to}/${to_path}" > to this: > > download_build "${ftp_server_from}/${from_path}" > > download_build "${ftp_server_to}/${to_path}" > > If I leave the removal inside of download_build, the first download is as it > would have never happened. > I can add a do_not_clobber parameter and set to True on the first call if > you prefer to. Ah, right. This is totally fine, I just lost context on it. Thanks for the reminder! > > > @@ +167,5 @@ > > > + # Make sure there is nothing lingering left from previous runs. > > > + # We're hoping to improve this in firefox-ui-tests > > > + # Bug https://bugzilla.mozilla.org/show_bug.cgi?id=1152460 > > > + if [ `netstat -anp 2>/dev/null | grep ':2828.*127' | grep TIME_WAIT | wc -l` -ne 0 ] > > > + then > > > > Has this been tested on all platforms yet? I can't r+ unless you know it > > works.... > > This will be removed in the final product. > It's worked though locally for me on Windows and Mac. Okay...can you remove it from the next patch you post in that case? If it's in a patch that I'm reviewing, I can only assume that it's something you want to land with the rest of the patch.

Armen [:armenzg]

Assignee

Comment 59

•

10 years ago

Attached patch [tools] Add marionette update testing mode (obsolete) — Details — Splinter Review

Is this heading in the right direction? It's been working on the Linux loaner. I will have to do further testing on Windows and Mac and using the --developer-mode flag.

Attachment #8594969 - Attachment is obsolete: true

Attachment #8596676 - Flags: feedback?(bhearsum)

Chris Manchester (limited bugmail, email directly)

Updated

•

10 years ago

Depends on: 1157823

Armen [:armenzg]

Assignee

Updated

•

10 years ago

Depends on: 1154060

Armen [:armenzg]

Assignee

Comment 60

•

10 years ago

Attached patch [tools] Add marionette update testing mode (obsolete) — Details — Splinter Review

I believe I have addressed all of your suggestions. I've updated the documentation: https://wiki.mozilla.org/Auto-tools/Projects/Marionette_update_tests

Attachment #8596676 - Attachment is obsolete: true

Attachment #8596676 - Flags: feedback?(bhearsum)

Attachment #8596823 - Flags: review?(bhearsum)

Chris Manchester (limited bugmail, email directly)

Updated

•

10 years ago

Depends on: 1157902

Armen [:armenzg]

Assignee

Updated

•

10 years ago

Depends on: 1157665

Armen [:armenzg]

Assignee

Comment 61

•

10 years ago

Our latest run was running pefectly until we run out of space. [1] The space issue will be solved once we start using marionette driver 0.5 (bug 1157665). The firefox-ui-tests change from bug 1156475 was being tested on this run and has been proven useful. Hence, removing dependency on bug 1141519. [1] armenzg@armenzg-thinkpad:~$ grep "Dumping good" fullrun.txt | wc -l 252 armenzg@armenzg-thinkpad:~$ grep "Dumping bad" fullrun.txt | wc -l 2 [cltbld@dev-linux64-ec2-armenzg.dev.releng.use1.mozilla.com tmp]$ du -sh /tmp 16G /tmp [cltbld@dev-linux64-ec2-armenzg.dev.releng.use1.mozilla.com tmp]$ ls -l /tmp | grep "moz" | wc -l 509 [cltbld@dev-linux64-ec2-armenzg.dev.releng.use1.mozilla.com tmp]$ rm -rf /tmp/*mozrunner [cltbld@dev-linux64-ec2-armenzg.dev.releng.use1.mozilla.com tmp]$ ls -l /tmp | grep "moz" | wc -l 0 [cltbld@dev-linux64-ec2-armenzg.dev.releng.use1.mozilla.com tmp]$ du -sh /tmp 249M /tmp

No longer depends on: 1141519

bhearsum@mozilla.com (:bhearsum)

Comment 62

•

10 years ago

Comment on attachment 8596823 [details] [diff] [review] [tools] Add marionette update testing mode Review of attachment 8596823 [details] [diff] [review]: ----------------------------------------------------------------- r-'ing for the pypi mirror parts, and possibly the requirements file (depending why it's there) ::: release/common/firefox_ui_updates_requirements.txt @@ +1,1 @@ > +firefox-ui-tests==0.2 Is this file actually necessary? It seems like running "pip install firefox-ui-tests" should give you all the right packages, assuming "firefox-ui-tests" has its own requirements defined correctly. ::: release/common/setup_firefox_ui_updates.sh @@ +1,5 @@ > +#!/bin/bash > +# This script creates a virtualenv with all the dependencies to run > +# firefox-ui-update tests. > +DIR=$( cd "$( dirname "${BASH_SOURCE[0]}" )" && pwd ) > +PYWIN32=http://pypi.pub.build.mozilla.org/pub/pywin32-216.win32-py2.7.exe I don't understand why you this is still pointing at specific pypi mirrors. On RelEng machines, pip.conf will point to a bunch of mirrors. On a developer's machine, it will point at pypi.python.org, or whatever they've configured their own pypi to use. I don't see any reason to override this in either scenario. Pointing at specific mirrors like this is a time bomb. If I'm missing something here, please let me know. ::: release/updates/verify.sh @@ +34,5 @@ > echo " -c, --complete complete upgrade test" > + echo " --marionette test the new marionette approach" > + echo " --dont-clear-cache do not clear the cache" > + echo " --keep-venv if you do not want the venv to be removed" > + echo " --developer-mode if you're not running this on a releng loaner" As I said before, I really don't like developer mode being implemented as args to this script. A wrapper would be much more appropriate. But since this stuff has to happen after the virtualenv is created, I guess you have no choice.

Attachment #8596823 - Flags: review?(bhearsum) → review-

Armen [:armenzg]

Assignee

Comment 63

•

10 years ago

(In reply to Ben Hearsum [:bhearsum] from comment #62) > Comment on attachment 8596823 [details] [diff] [review] > [tools] Add marionette update testing mode > > Review of attachment 8596823 [details] [diff] [review]: > ----------------------------------------------------------------- > > r-'ing for the pypi mirror parts, and possibly the requirements file > (depending why it's there) > > ::: release/common/firefox_ui_updates_requirements.txt > @@ +1,1 @@ > > +firefox-ui-tests==0.2 > > Is this file actually necessary? It seems like running "pip install > firefox-ui-tests" should give you all the right packages, assuming > "firefox-ui-tests" has its own requirements defined correctly. > firefox-ui-tests sets the requirements in here: https://github.com/mozilla/firefox-ui-tests/blob/master/setup.py#L9 Some of those packages are not hard pinned: http://hg.mozilla.org/mozilla-central/file/86d3308ec888/testing/marionette/client/requirements.txt The reason I'm even considering the requirements.txt is to ensure that when a new package is uploaded to our internal pypi we don't have unintentionally new versions of these packages. It would be almost invisible to releng unless inspecting the logs. What would be a good approach to this? Define all of the dependencies on firefox-ui-tests? chmanchester: would this block above be OK with you? > ::: release/common/setup_firefox_ui_updates.sh > @@ +1,5 @@ > > +#!/bin/bash > > +# This script creates a virtualenv with all the dependencies to run > > +# firefox-ui-update tests. > > +DIR=$( cd "$( dirname "${BASH_SOURCE[0]}" )" && pwd ) > > +PYWIN32=http://pypi.pub.build.mozilla.org/pub/pywin32-216.win32-py2.7.exe > > I don't understand why you this is still pointing at specific pypi mirrors. > On RelEng machines, pip.conf will point to a bunch of mirrors. On a > developer's machine, it will point at pypi.python.org, or whatever they've > configured their own pypi to use. I don't see any reason to override this in > either scenario. Pointing at specific mirrors like this is a time bomb. > > If I'm missing something here, please let me know. > That is *only* used when someone uses --developer-mode since win32api does not come as part of mozilla-build. "pip install" does not work for this package. I will look closer into why. Of what I'm sure is that we use "easy_install" instead of "pip" for it on mozharness [1] It's also installed in such way by puppet [2] I've also documented some of it in here [3] since I had no luck finding proper documentation anywhere. [1] http://hg.mozilla.org/build/mozharness/file/default/mozharness/base/python.py#l391 [2] http://mxr.mozilla.org/build/source/puppet/modules/packages/manifests/pywin32.pp#11 [3] http://armenzg.blogspot.ca/2015/04/how-to-install-pywin32-on-windows.html > ::: release/updates/verify.sh > @@ +34,5 @@ > > echo " -c, --complete complete upgrade test" > > + echo " --marionette test the new marionette approach" > > + echo " --dont-clear-cache do not clear the cache" > > + echo " --keep-venv if you do not want the venv to be removed" > > + echo " --developer-mode if you're not running this on a releng loaner" > > As I said before, I really don't like developer mode being implemented as > args to this script. A wrapper would be much more appropriate. But since > this stuff has to happen after the virtualenv is created, I guess you have > no choice. When you talk about a wrapper, are you talking about a small script that would do something like this: ../common/setup_firefox_ui_updates.sh --developer-mode venv ./verify.sh --marionette mozBeta-firefox-linux64.cfg --keep-venv At that point, I would not need to specify --developer-mode inside of verify.sh. I could also teach setup_firefox_ui_updates.sh to read an env, e.g.: DEV_MODE=1 ./verify.sh --marionette mozBeta-firefox-linux64.cfg

Flags: needinfo?(cmanchester)

bhearsum@mozilla.com (:bhearsum)

Comment 64

•

10 years ago

(In reply to Armen Zambrano G. (:armenzg - Toronto) from comment #63) > (In reply to Ben Hearsum [:bhearsum] from comment #62) > > Comment on attachment 8596823 [details] [diff] [review] > > [tools] Add marionette update testing mode > > > > Review of attachment 8596823 [details] [diff] [review]: > > ----------------------------------------------------------------- > > > > r-'ing for the pypi mirror parts, and possibly the requirements file > > (depending why it's there) > > > > ::: release/common/firefox_ui_updates_requirements.txt > > @@ +1,1 @@ > > > +firefox-ui-tests==0.2 > > > > Is this file actually necessary? It seems like running "pip install > > firefox-ui-tests" should give you all the right packages, assuming > > "firefox-ui-tests" has its own requirements defined correctly. > > > firefox-ui-tests sets the requirements in here: > https://github.com/mozilla/firefox-ui-tests/blob/master/setup.py#L9 > > Some of those packages are not hard pinned: > http://hg.mozilla.org/mozilla-central/file/86d3308ec888/testing/marionette/ > client/requirements.txt > > The reason I'm even considering the requirements.txt is to ensure that when > a new package is uploaded to our internal pypi we don't have unintentionally > new versions of these packages. Alright, I think it makes sense to keep the requirements file in this case. It would be silly to pin the overall firefox-ui-test module to versions that only make sense when running in the RelEng network. > > ::: release/common/setup_firefox_ui_updates.sh > > @@ +1,5 @@ > > > +#!/bin/bash > > > +# This script creates a virtualenv with all the dependencies to run > > > +# firefox-ui-update tests. > > > +DIR=$( cd "$( dirname "${BASH_SOURCE[0]}" )" && pwd ) > > > +PYWIN32=http://pypi.pub.build.mozilla.org/pub/pywin32-216.win32-py2.7.exe > > > > I don't understand why you this is still pointing at specific pypi mirrors. > > On RelEng machines, pip.conf will point to a bunch of mirrors. On a > > developer's machine, it will point at pypi.python.org, or whatever they've > > configured their own pypi to use. I don't see any reason to override this in > > either scenario. Pointing at specific mirrors like this is a time bomb. > > > > If I'm missing something here, please let me know. > > > That is *only* used when someone uses --developer-mode since win32api does > not come as part of mozilla-build. > "pip install" does not work for this package. > I will look closer into why. None of this explains why you're pointing at specific mirrors. This is only used in developer mode, so why isn't "easy_install pywin32" good enough? easy_install will use whatever mirror the developer's machine is configured to use, which seems a lot better than hard coding a host with no guarantees about its lifetime. > > ::: release/updates/verify.sh > > @@ +34,5 @@ > > > echo " -c, --complete complete upgrade test" > > > + echo " --marionette test the new marionette approach" > > > + echo " --dont-clear-cache do not clear the cache" > > > + echo " --keep-venv if you do not want the venv to be removed" > > > + echo " --developer-mode if you're not running this on a releng loaner" > > > > As I said before, I really don't like developer mode being implemented as > > args to this script. A wrapper would be much more appropriate. But since > > this stuff has to happen after the virtualenv is created, I guess you have > > no choice. > > When you talk about a wrapper, are you talking about a small script that > would do something like this: > ../common/setup_firefox_ui_updates.sh --developer-mode venv > ./verify.sh --marionette mozBeta-firefox-linux64.cfg --keep- > > At that point, I would not need to specify --developer-mode inside of > verify.sh. Yes, this is *exactly* what I was thinking! This would be much better IMO. > I could also teach setup_firefox_ui_updates.sh to read an env, e.g.: > DEV_MODE=1 ./verify.sh --marionette mozBeta-firefox-linux64.cfg

Chris Manchester (limited bugmail, email directly)

Comment 65

•

Assignee

Updated

•

10 years ago

Depends on: 1160595

Henrik Skupin [:whimboo][⌚️UTC+2]

Comment 68

•

10 years ago

I would like to add here that it is not clear yet how we can best use the firefox-ui-tests code. Right now we only have a single branch for mozilla-central, but that will change in a bit. It means we will have branches for Nightly, Aurora, Beta, Release, and ESR. The distribution story via pypi will not work that way. Similar to Mozmill we will have to best clone the git repository to get the code. This is also preferred so we do not have to release new versions all over the place even for the tiniest change in the tests. This would be hilarious. Otherwise Armen and I will have a chat around Thursday where we will talk about still outstanding issues and questions.

Status: NEW → ASSIGNED

OS: Linux → All

Hardware: x86_64 → All

Armen [:armenzg]

Assignee

Comment 69

•

10 years ago

Are you suggesting to maintain various branches of firefox-ui-tests? bhearsum: I think we have to rewrite my whole work on mozharness. It is getting ridicolous how much work I have needed to put to figure out how to setup everything just so for each platform. I still have not fixed Windows and whimboo's suggestion to checkout might be needed. I don't think we have git on the Windows builders since I had to install it myself. What would be our alternative? diff --git a/release/common/setup_firefox_ui_updates.sh b/release/common/setup_f index f97613d..d41ed4e 100755 --- a/release/common/setup_firefox_ui_updates.sh +++ b/release/common/setup_firefox_ui_updates.sh @@ -76,17 +76,20 @@ fi # Activate virtualenv if [[ "`uname`" == $WIN32_UNAME ]] then - source $venv_dir/Scripts/activate + python $venv_dir/Scripts/activate_this.py + pip_options="--no-index --find-links http://pypi.pvt.build.mozilla.org/pub" + pip_path="$venv_dir/Scripts/pip" else source $venv_dir/bin/activate + pip_path="pip" fi # firefox-ui-tests has few requirements, however, its requirements do not have # hard pinned versions. If a new version of those dependencies would be uploade # to the internal pypi, we would start using it unless we hard pinned them here -pip install $pip_options -r $DIR/firefox_ui_updates_requirements.txt || exit +$pip_path install $pip_options -r $DIR/firefox_ui_updates_requirements.txt || # We install firefox-ui-tests separetedly to cleary distinct the main tool from -pip install $pip_options firefox-ui-tests==0.2 || exit +$pip_path install $pip_options firefox-ui-tests==0.2 || exit # Most local Windows machines don't have win32api installed if [ $developer_mode ] && [[ "`uname`" == $WIN32_UNAME ]] @@ -95,4 +98,4 @@ then easy_install $PYWIN32 fi -pip freeze +$pip_path freeze

Armen [:armenzg]

Assignee

Comment 70

•

10 years ago

There is C:/mozilla-build/Git/bin

bhearsum@mozilla.com (:bhearsum)

•

10 years ago

Todo: * Ensure that we can specify the right tag of the tools repo

Henrik Skupin [:whimboo][⌚️UTC+2]

Comment 77

•

10 years ago

That's great Armen! Some more things to note... the update tests allow to specify some more options. Please see all the --update-* options of the "firefox-ui-update --help" command. What you definitely also need is --update-allow-mar-channel, --update-target-version, and --update-target-buildid. The latter two are for final checks that Firefox has been updated to the correct target version. The MAR channel option is for updating a release build to a beta build, which happens for RC builds on the beta channel.

Armen [:armenzg]

Assignee

Comment 78

•

10 years ago

Thanks Henrik! We can pass all other non-explicitley defined options down to the binary if we also want to. Would you mind adding some examples for those? It would help me be sure I understand them. I've definitely lost touch with our releases :( Side note, last time I checked firefox-ui-updates had few more options from the other binary that did not apply to it (related to adb, I think; I can't remember).

Henrik Skupin [:whimboo][⌚️UTC+2]

Comment 79

•

10 years ago

The only really important options are prefixed with `--update` and are located at the very end of the help output. Here the two examples: 1. Ensure that the update has been applied correctly by checking the target version and build id ./firefox-ui-update --installer %Firefox38.0b8% --update-target-version=38.0b9 --update-target-buildid=20150429135941. Even if the tests pass without those options defined, it does not mean that the update was successful. You really want to check that Firefox got updated to the target build. 2. A beta user got updated to a RC build and has to be brought back to a beta build. Therefore you need --update-allow-mar-channel Lets say a beta user was on 38.0b9 and got updated to 38.0RC. This RC build is on the release channel, but beta users have to be brought back to beta channel. Given that we currently cannot test updates in multiple steps vXb9 -> RC -> v(X+1)b1, we have to run vXb9 -> RC, and RC -> v(X+1)b1 separately. Using the RC build as source build only has the release channel active. The option above can also enable the beta channel via update-settings.ini. ./firefox-ui-update --installer %Firefox38.0% --update-target-version=39.0b1 --update-target-buildid=%whateveritwillbe% --update-allow-mar-channel=firefox-mozilla-beta

Armen [:armenzg]

Assignee

Comment 80

•

10 years ago

Attached patch Create firefox UI updates script (obsolete) — Details — Splinter Review

Hi Ben, I have a script that works for a Linux loaner. I have not yet added configs for various platforms. You can see below things that are left to be added. I'm looking in here things that you want me to address for the final release. I'm not looking for a full review. ########################## Status summary: Accomplished: * Added mechanism to add extra harness options * Added support for --installer-path Todo: * Add configs for platforms * Test on Windows and Mac loaner ** This would lead to creating specific config for the three platforms ** We probably want these in the tree to ensure riding the trains * Add support for chunking releases * Ensure that we can specify the right tag of the tools repo Not blockers: * Add proxxy support to speed up downloads * When using --installer-url ask the user to set the --firefox-ui-branch

Attachment #8599866 - Attachment is obsolete: true

Attachment #8608330 - Flags: feedback?(bhearsum)

Armen [:armenzg]

Assignee

Comment 81

•

10 years ago

Ben, why do we have duplicated entries on the updates configs? (for the same build id) Specifically, the ones that don't have "ftp_server_from" defined.

bhearsum@mozilla.com (:bhearsum)

•

10 years ago

(In reply to Ben Hearsum [:bhearsum] from comment #83) > Comment on attachment 8608330 [details] [diff] [review] > Create firefox UI updates script > > Review of attachment 8608330 [details] [diff] [review]: > ----------------------------------------------------------------- > > Overall notes: > * Now that I'm seeing them, I'm probably not a good reviewer for the > developer mode parts. I thought you were going to separate those out into > another patch anyways? It's a pain to separate them :P I can probably not be lazy and separate them. > * The script looks mostly fine, but I'd like to see you make use of the > existing update verify config parser that we have, and unify the > installer_path/installer_url/update_config modes a bit. More on these below. > > ::: mozharness/mozilla/testing/firefox_ui_tests.py > @@ +69,5 @@ > > + ''' > > + dirs = self.query_abs_dirs() > > + > > + self.vcs_checkout( > > + repo='https://github.com/mozilla/firefox-ui-tests.git', > > This should be a config option with this as the default, not hardcoded. > Correct. > ::: scripts/firefox_ui_updates.py > @@ +1,1 @@ > > +#!/usr/bin/env python > > Something that would help simplify this script a lot would be to unify the > installer_url/installer_path/update_config modes as much as possible. If > determine-testing-configuration _always_ set self.releases, nothing that > happens after it would need any special cases, nor would the _pre_run_tests > method. More on this below. > I have considered of building a releases structured based on self.installer* info. > @@ +51,5 @@ > > + [['--tools-revision'], { > > + 'dest': 'tools_revision', > > + 'help': 'which revision/tag to use for tools', > > + }], > > + [['--update-config-file'], { > > --update-verify-config-file, please. Calling it "update config" makes it > sound like it's something different than the "update verify configs" that we > already have. > Sure. > @@ +132,5 @@ > > + vcs='hgtool' > > + ) > > + > > + > > + def determine_testing_configuration(self): > > I strongly advise you not to use your own parser here. I'm sure it's working > fine with your test cases, but there's probably edge cases you haven't > considered. We have a class that knows how to properly parse update verify > configs, and will even handle chunking for you: > https://github.com/mozilla/build-tools/blob/master/lib/python/release/ > updates/verify.py. > Since I check out tools, I should be able to use it. Let's see. > @@ +157,5 @@ > > + to="/firefox/candidates/38.0-candidates/build2/linux-x86_64/%locale%/firefox-38.0.tar.bz2" > > + > > + We will store this information in self.releases as a list of dict per release. > > + ''' > > + self.releases = [] > > This should probably be set as the default in __init__. If someone were to > run this script without this step included in the actions, run_tests would > end up raising an error because releases is None. > OK. > @@ +255,5 @@ > > + if self.installer_url: > > + self.installer_path = self.download_file( > > + self.installer_url, > > + parent_dir=dirs['abs_work_dir'] > > + ) > > This, and the url creation code in the block below, should probably move to > determine-update-configuration. If self.releases contained an URL already, > there would be no need for this special case here. > Hopefully. > @@ +258,5 @@ > > + parent_dir=dirs['abs_work_dir'] > > + ) > > + > > + if self.installer_path: > > + self._run_test(self.installer_path) > > This special case can be removed as well if installer_path was defined in > self.releases, and you checked it for existence before downloading. > Sounds good. > @@ +271,5 @@ > > + > > + installer_path = self.download_file( > > + url=url, > > + parent_dir=dirs['abs_work_dir'] > > + ) > > One thing you've lost in your conversion to Mozharness is the caching that > update verify does. I'm not going to r- initially on this, but it is a > requirement for chunking because of how much additional load it generates. I've thought about it but I can't think of a case where we would actually download twice the same installer even when we did chunking. Nevertheless, for mozharness we should add the ability to do caching if wanted. (In reply to Ben Hearsum [:bhearsum] from comment #84) > (In reply to Armen Zambrano G. (:armenzg - Toronto) from comment #80) > > Created attachment 8608330 [details] [diff] [review] > > Create firefox UI updates script > > > > Hi Ben, > > I have a script that works for a Linux loaner. > > I have not yet added configs for various platforms. > > You can see below things that are left to be added. > > I'm looking in here things that you want me to address for the final release. > > I'm not looking for a full review. > > > > > > ########################## > > Status summary: > > > > Accomplished: > > * Added mechanism to add extra harness options > > * Added support for --installer-path > > > > Todo: > > * Add configs for platforms > > * Test on Windows and Mac loaner > > ** This would lead to creating specific config for the three platforms > > ** We probably want these in the tree to ensure riding the trains > > What exactly is going in these configs? The script is almost entirely driven > by the update verify config (which is a good thing!). > For instance, for Linux we need to set DISPLAY=:2 That kind of configuration. > > > * Add proxxy support to speed up downloads > > As noted in my review, caching *is* a blocker when we're chunking. And > chunking is probably a blocker if we want these to be run with every release > (otherwise they'll take forever). > Currently update verify does caching but does not use proxy capacities if I understand correctly. As mentioned above, I don't think caching is needed if we're going to run this side by side with the other update verify jobs. I also can't think of this script downloading the same installer twice on one machine. If you can think of it please let me know. I think we can look at what else is needed to bring update verify into this script in a following quarter but for now I think we're fine. > > * When using --installer-url ask the user to set the --firefox-ui-branch > > Why can't this just be another argument? It's entirely possible that we may > need to set the branch in other modes too. Eg: if esr and betas need to use > different firefox-ui code. I wanted to make sure that the right branch is used for the right installer. Otherwise, people might report issues when they are using a different branch with different firefox-ui-tests requirements.

bhearsum@mozilla.com (:bhearsum)

Comment 86

•

10 years ago

(In reply to Armen Zambrano G. (:armenzg - Toronto) from comment #85) > > ::: scripts/firefox_ui_updates.py > > @@ +1,1 @@ > > > +#!/usr/bin/env python > > > > Something that would help simplify this script a lot would be to unify the > > installer_url/installer_path/update_config modes as much as possible. If > > determine-testing-configuration _always_ set self.releases, nothing that > > happens after it would need any special cases, nor would the _pre_run_tests > > method. More on this below. > > > I have considered of building a releases structured based on self.installer* > info. I think that's a fantastic idea. If you can build a structure that looks the same regardless of whether you got your data from an update verify config or from one of the other arguments, the rest of the code is much cleaner! > > @@ +271,5 @@ > > > + > > > + installer_path = self.download_file( > > > + url=url, > > > + parent_dir=dirs['abs_work_dir'] > > > + ) > > > > One thing you've lost in your conversion to Mozharness is the caching that > > update verify does. I'm not going to r- initially on this, but it is a > > requirement for chunking because of how much additional load it generates. > > I've thought about it but I can't think of a case where we would actually > download twice the same installer even when we did chunking. > Nevertheless, for mozharness we should add the ability to do caching if > wanted. > > > * Add proxxy support to speed up downloads > > > > As noted in my review, caching *is* a blocker when we're chunking. And > > chunking is probably a blocker if we want these to be run with every release > > (otherwise they'll take forever). > > > Currently update verify does caching but does not use proxy capacities if I > understand correctly. Hm, you're right about this. Update verify's caching focus on the "to" MARs, which it downloads by hand. We can't do that here because Firefox is the one downloading...I guess that's why you mentioned proxxy. We'll definitely want _that_ instead... > As mentioned above, I don't think caching is needed if we're going to run > this side by side with the other update verify jobs. I also can't think of > this script downloading the same installer twice on one machine. If you can > think of it please let me know. > Totally agree on the "from" package side -- caching those doesn't help. Can we use proxxy to cache the MARs that the test downloads though? _Those_ will be redownloaded many, many times. I'm worried about the load this is going to generate once we start chunking it. > I think we can look at what else is needed to bring update verify into this > script in a following quarter but for now I think > we're fine. Just to be clear: I'm not asking you to port update verify, just to make sure that this script doesn't generate more load than necessary. > > > * When using --installer-url ask the user to set the --firefox-ui-branch > > > > Why can't this just be another argument? It's entirely possible that we may > > need to set the branch in other modes too. Eg: if esr and betas need to use > > different firefox-ui code. > > I wanted to make sure that the right branch is used for the right installer. > Otherwise, people might report issues when they are using a different branch > with different firefox-ui-tests requirements. Oh, I think I misunderstood. I was reading the original comment as "prompt the user interactively", but upon a reread it reads more like "require the --firefox-ui-branch argument of --installer-url is set". If that's the case, we're on the same page.

Armen [:armenzg]

Assignee

Comment 87

•

10 years ago

(In reply to Ben Hearsum [:bhearsum] from comment #86) > > As mentioned above, I don't think caching is needed if we're going to run > > this side by side with the other update verify jobs. I also can't think of > > this script downloading the same installer twice on one machine. If you can > > think of it please let me know. > > > > Totally agree on the "from" package side -- caching those doesn't help. Can > we use proxxy to cache the MARs that the test downloads though? _Those_ will > be redownloaded many, many times. I'm worried about the load this is going > to generate once we start chunking it. > Do you have any suggestion on how? I've thought about it but it is the browser that grabs the files. Proxxy is not a real proxy but simply hijacks (redirects) mozharness downloads. If we could do this, I believe it would be the single top improvement for this harness (that and faster restarts of the browser :P).

Armen [:armenzg]

Assignee

Comment 88

•

10 years ago

(In reply to Ben Hearsum [:bhearsum] from comment #86) > (In reply to Armen Zambrano G. (:armenzg - Toronto) from comment #85) > > > ::: scripts/firefox_ui_updates.py > > > @@ +1,1 @@ > > > > +#!/usr/bin/env python > > > > > > Something that would help simplify this script a lot would be to unify the > > > installer_url/installer_path/update_config modes as much as possible. If > > > determine-testing-configuration _always_ set self.releases, nothing that > > > happens after it would need any special cases, nor would the _pre_run_tests > > > method. More on this below. > > > > > I have considered of building a releases structured based on self.installer* > > info. > > I think that's a fantastic idea. If you can build a structure that looks the > same regardless of whether you got your data from an update verify config or > from one of the other arguments, the rest of the code is much cleaner! > I've tried this and it is not looking pretty with using regexes against an installer_url or installer_path. I will try again later but for now I would like to shelve it and perhaps request not to be part of the review needs.

Armen [:armenzg]

Assignee

Comment 89

•

10 years ago

Attached patch Core mozharness changes (obsolete) — Details — Splinter Review

Additions to developer config: * Add hgtool and gittool to the exes * Add URLs to hgtool and gittools Add $work_dir/venv as the default value for VirtualenMixin ScriptMixin to read first from "exes" dict if defined. Created mozharness/mozilla/vcstools.py in order to fetch gittool and hgtool when using developer mode.

Armen [:armenzg]

Assignee

Comment 90

•

10 years ago

Attachment #8610248 - Flags: feedback?(bhearsum)

Armen [:armenzg]

Assignee

Comment 98

•

10 years ago

Status summary ############## Previous update on comment 80. Accomplished: * Ensure that we can specify the right tag of the tools repo * Add support for chunking releases * Switched to UpdateVerifyConfig as the parser * Added dry-run to show information about releases being tested ** We probably should add the information always Todo: * Add configs for platforms * Test on Windows and Mac loaner ** This would lead to creating specific config for the three platforms ** We probably want these in the tree to ensure riding the trains Not blockers: * Add proxxy support to speed up downloads * When using --installer-url ask the user to set the --firefox-ui-branch

Henrik Skupin [:whimboo][⌚️UTC+2]

Comment 99

•

10 years ago

Regarding the final testing matrix we had a discussion in Portland last year and came up with a solution which has been noted here: https://github.com/mozilla/mozmill-ci/issues/535

bhearsum@mozilla.com (:bhearsum)

Comment 100

•

10 years ago

Comment on attachment 8610248 [details] [diff] [review] fx_mozharness.diff Review of attachment 8610248 [details] [diff] [review]: ----------------------------------------------------------------- ::: scripts/firefox_ui_updates.py @@ +108,5 @@ > + dirs['tools_dir'], 'release', 'updates', > + self.config['update_verify_config'] > + ) > + self.tools_verify = os.path.join( > + dirs['tools_dir'], 'release', 'updates', 'verify.py' This path looks wrong to me. This file is in tools/lib/python/release/updates/verify.py, not tools/release/updates. @@ +208,5 @@ > + # We need to keep the original update verify config instance untouched > + original_releases = uvc.releases > + # We need to get rid of the full releases before chunking > + # otherwise we do chunking with reduced sets of locales and duplicate release info > + uvc.releases = uvc.getQuickReleaseTests() So, now you're using only the quick releases, which means you're going to miss de, en-US, and ru for most old releases. You must use the locales from _both_ quick and full in order to get full coverage. Or you could just use a hack and add those three to every release that doesn't have them.

Attachment #8610248 - Flags: feedback?(bhearsum) → feedback+

Armen [:armenzg]

Assignee

Jonathan Griffin (:jgriffin)

Comment 106

•

Assignee

Comment 112

•

10 years ago

Comment on attachment 8608913 [details] [diff] [review] Core mozharness changes Review of attachment 8608913 [details] [diff] [review]: ----------------------------------------------------------------- Please let me know what you prefer so I can adjust my code. ::: mozharness/base/python.py @@ +100,5 @@ > virtualenv = os.path.join(dirs['abs_work_dir'], > c['virtualenv_path']) > + else: > + # If no value has been set use a default dir > + virtualenv = os.path.join(dirs['abs_work_dir'], 'venv') I don't need this change anymore since I'm adding venv path to the config file. This was so I would not need to create a config just to specify a venv dir. ::: mozharness/base/script.py @@ +507,5 @@ > if is_exe(program): > return program > else: > + # If the exe file is defined in the configs let's use that > + exe = self.config.get('exes', {}).get(program) I guess I did not think of it! What should we do about it? Should we make which call query_exe? ::: mozharness/mozilla/vcstools.py @@ +13,5 @@ > +from mozharness.base.script import PreScriptAction > +from mozharness.base.vcs.vcsbase import VCSScript > + > + > +class VCSToolsScript(VCSScript): Tc_vcs requires installing node. The node installation is not something we have investigated for Windows and Mac. I'm going to stick with the evil we know if you don't mind. I don't think that they have diverged but the classes {hg,git}tool.py require the _scripts_ {hg,git}tool.py to be present on the host, which is the case for Linux and maybe Mac. Look what I need to do on Windows: https://github.com/armenzg/build-mozharness/blob/update_testing/mozharness/mozilla/testing/firefox_ui_tests.py#L31 I'm lucky that the tools repo is checked out on them. Otherwise I would need to do the same as in developer mode. Would you like us to check-in the self-contained {hg,git}tool.py _scripts_ that are hosted on puppet to be checked-in inside of external_tools/ ? Also note, that the self-contained scripts are actually generated from the tools repo so people don't have to checkout it out. See this line on them: module_sources = [('util', 'eJxlkMEKgzAQRO/5isWTQhFaSg8Ff6LnQknM2ixoItmov1+T2F ......................... http://hg.mozilla.org/build/puppet/file/faaf5abd792e/modules/packages/files/hgtool.py#l3 and that is because of needing the lib/python under tools/ sys.path.append(os.path.join(os.path.dirname(__file__), "../../lib/python")) http://hg.mozilla.org/build/tools/file/default/buildfarm/utils/hgtool.py#l12 There is also scripts/sourcetool.py which I don't know what the story is behind it.

Armen [:armenzg]

Assignee

Comment 113

•

10 years ago

I'm hitting an issue where a browser is left behind without updating: python scripts/firefox_ui_updates.py --installer-url http://stage.mozilla.org/pub/mozil la.org//firefox/releases/38.0/win32/ach/Firefox%20Setup%2038.0.exe This was noticed after running this: python scripts/firefox_ui_updates.py --update-verify-config mozRelease-firefox-win32.cfg I will paste the logs after the run completes in the morning.

Armen [:armenzg]

Assignee

Comment 114

•

10 years ago

Attached file 2nd test fails and the browser is not killed (obsolete) — Details

I noticed now that it is using --firefox-ui-branch master. I hope that is the issue but I would be surprised. The 2nd test fails and mozprocess times out. All other runs fail with busy 2828 port because the browser is still around. I believe fx_ui_tests should take care of killing the browser properly. I don't think the mozharness script should even though I could check. 18:59:29 INFO - 1:02.73 TEST_START: MainThread test_fallback_update.py TestFallbackUpdate.test_update 19:01:09 INFO - Automation Error: mozprocess timed out after 100 seconds running ['c:\\Users\\cltbld\\build-mozharness\\build\\venv\\Scripts\\firefox-ui-update', '--installer', 'c:\\Users\\cltbld\ 19:01:09 ERROR - timed out after 100 seconds of no output 19:01:09 ERROR - Return code: 572 19:01:09 INFO - == Dumping output of harness == 19:01:09 INFO - Reading from file c:\Users\cltbld\build-mozharness\build\harness.log 19:01:09 INFO - . 19:01:09 INFO - 19:01:09 INFO - 19:01:09 INFO - Ran 1 tests in 40.0s 19:01:09 INFO - == End of harness output == 19:01:09 WARNING - == Dumping gecko output == 19:01:09 INFO - Reading from file gecko.txt 19:01:09 ERROR - unable to open gecko.txt: No such file or directory 19:01:10 WARNING - == End of gecko output == 19:01:10 WARNING - FAIL: firefox-ui-update has failed. 19:01:10 INFO - You can run the following command to reproduce the issue: 19:01:10 INFO - python scripts/firefox_ui_updates.py --run-tests --installer-url http://stage.mozilla.org/pub/mozilla.org//firefox/releases/38.0/win32/ach/Firefox%20Setup%2038.0.exe --update-chann 19:01:10 INFO - If you want to run this on your development machine: 19:01:10 INFO - python scripts/firefox_ui_updates.py --cfg developer_config.py --installer-url http://stage.mozilla.org/pub/mozilla.org//firefox/releases/38.0/win32/ach/Firefox%20Setup%2038.0.exe

Attachment #8613517 - Flags: feedback?

Armen [:armenzg]

Assignee

Comment 115

•

10 years ago

This is a screen shot of the browser: http://grab.by/HI2i

Armen [:armenzg]

Assignee

Updated

•

•

10 years ago

(In reply to Henrik Skupin (:whimboo) from comment #120) > (In reply to Armen Zambrano G. (:armenzg - Toronto) from comment #119) > > For the record, I watched carefully and we might simply need to bump the > > timeout. > > Just before the background update finishes downloading the update we abort. > > > > I have left my last run > > So you are saying that the download of the update takes that long? It should > not be, because its a manual trigger and downloading the update should be > done really quickly. Maybe you hit a real slow CDN mirror? > I don't know if it hits stage or CDN. Side note, the update channel associated is release-localtest. > > We should nevertheless kill the browser rather than aborting like this. > > We currently give the download about 6 minutes of time AFAIK, which should > really be enough even on slow computers, and Windows is still the platform > with the smallest update package. It is mozprocess that times out after 100 seconds: 19:01:09 INFO - Automation Error: mozprocess timed out after 100 seconds running ['c:\\Users\\cltbld\\build-mozharness\\build\\venv\\Scripts\\firefox-ui-update', '--installer', 'c:\\Users\\cltbld\

Henrik Skupin [:whimboo][⌚️UTC+2]

Comment 122

•

10 years ago

(In reply to Armen Zambrano G. (:armenzg - Toronto) from comment #119) So I tried to run the script: > The first time: > * python scripts/firefox_ui_updates.py --installer-url > http://stage.mozilla.org/pub/mozilla.org//firefox/releases/38.0/win32/en-US/ > Firefox%20Setup%2038.0.exe --cfg generic_releng_config.py > --firefox-ui-branch mozilla-beta As you said on IRC I did it with the developer_config but then I still get an error. Here the traceback: 00:05:30 INFO - retry: Calling _download_file with args: ('http://hg.mozilla.org/build/puppet/raw-file/faaf5abd792e/modules/packages/files/hgtool.py', '/home/henrik/.mozilla/releng/hgtool.py'), kwargs: {}, attempt #1 00:05:31 ERROR - Exception during pre-action for checkout: Traceback (most recent call last): 00:05:31 ERROR - File "/mozilla/code/build-mozharness/mozharness/base/script.py", line 1229, in run_action 00:05:31 ERROR - method(action) 00:05:31 ERROR - File "/mozilla/code/build-mozharness/mozharness/mozilla/vcstools.py", line 33, in _pre_checkout 00:05:31 ERROR - file_name=file_path, 00:05:31 ERROR - File "/mozilla/code/build-mozharness/mozharness/base/script.py", line 325, in download_file 00:05:31 ERROR - status = self._retry_download_file(url, file_name, error_level, retry_config=retry_config) 00:05:31 ERROR - File "/mozilla/code/build-mozharness/mozharness/base/script.py", line 303, in _retry_download_file 00:05:31 ERROR - **retry_args 00:05:31 ERROR - File "/mozilla/code/build-mozharness/mozharness/base/script.py", line 587, in retry 00:05:31 ERROR - status = action(*args, **kwargs) 00:05:31 ERROR - File "/mozilla/code/build-mozharness/mozharness/base/script.py", line 241, in _download_file 00:05:31 ERROR - local_file = open(file_name, 'wb') 00:05:31 ERROR - IOError: [Errno 2] No such file or directory: '/home/henrik/.mozilla/releng/hgtool.py' 00:05:31 FATAL - Aborting due to exception in pre-action listener. 00:05:31 FATAL - Running post_fatal callback... 00:05:31 FATAL - Exiting -1

Chris Manchester (limited bugmail, email directly)

Comment 123

•

10 years ago

Comment on attachment 8613517 [details] 2nd test fails and the browser is not killed After discussing on IRC it appears this needs a longer timeout in mozharness. Armen offered to take it from here.

Attachment #8613517 - Flags: feedback?(cmanchester)

Armen [:armenzg]

Assignee

Comment 124

•

10 years ago

Attached patch [mozharness] Core changes: clean up dev config, add VCSToolsScript for support of hgtool and gittool on developer mode, create PlatfromMixin (obsolete) — Details — Splinter Review

For PlatformMixin we actually use self: http://hg.mozilla.org/build/mozharness/file/default/mozharness/base/script.py#l89

Attachment #8608913 - Attachment is obsolete: true

Attachment #8614132 - Flags: review?(jlund)

Armen [:armenzg]

Assignee

Comment 125

•

10 years ago

Attached patch fx_mozharness.diff (obsolete) — Details — Splinter Review

Attachment #8592267 - Attachment is obsolete: true

Attachment #8592491 - Attachment is obsolete: true

Attachment #8610248 - Attachment is obsolete: true

Attachment #8610804 - Attachment is obsolete: true

Attachment #8611377 - Attachment is obsolete: true

Attachment #8613517 - Attachment is obsolete: true

Attachment #8614137 - Flags: review?(bhearsum)

Armen [:armenzg]

Assignee

Comment 126

•

10 years ago

I've tested it on all the releng build platforms. On a following push, I've renamed these: renamed: configs/update_tests/mozilla-beta_updates_config.py -> configs/update_tests/mozilla-beta.py renamed: configs/update_tests/mozilla-release_updates_config.py -> configs/update_tests/mozilla-release.py The syntax is like this: python scripts/firefox_ui_updates.py --cfg generic_releng_config.py --cfg update_tests/mozilla-beta.py python scripts/firefox_ui_updates.py --cfg generic_releng_config.py --cfg update_tests/mozilla-release.py It auto-detects the platform being run on and it selects the right tools update verify config. You can run with --dry-run True to not run the whole thing. I decided to use configs in order to make the same configs regardless of the platform and I've stored in them the right firefox_ui_tests branch to use.

Armen [:armenzg]

Assignee

Comment 127

•

10 years ago

Attached patch [mozharness] Core changes: clean up dev config, add VCSToolsScript for support of hgtool and gittool on developer mode, create PlatfromMixin, plus create dir for hgtool/gittool — Details — Splinter Review

I've added some more information to help when someone runs without --cfg developer_config.py I also fixed the issue that whimboo hit earlier. Not everybody has $HOME/.mozilla/releng created on their machines.

Attachment #8614132 - Attachment is obsolete: true

Attachment #8614132 - Flags: review?(jlund)

Attachment #8614348 - Flags: review?(jlund)

Armen [:armenzg]

Assignee

Comment 128

•

10 years ago

Attached patch [mozharness] Firefox UI update tests — Details — Splinter Review

The interdiff is minimal. I improved generic_releng_config.py to include explicit paths to {hg,git}tool.py on Linux and Mac since /usr/local/bin just happened to include it on PATH.

Attachment #8614137 - Attachment is obsolete: true

Attachment #8614137 - Flags: review?(bhearsum)

Attachment #8614349 - Flags: review?(bhearsum)

Henrik Skupin [:whimboo][⌚️UTC+2]

Comment 129

•

10 years ago

Armen, I tried the developer_config locally now again and it works great. Something which is strange is the final assert that not all windows have been closed. I do not see it when I run the tests directly outside of mozharness. I may have to check again if something has been changed lately.

bhearsum@mozilla.com (:bhearsum)

Updated

•

10 years ago

Attachment #8614349 - Flags: review?(bhearsum) → review+

Jordan Lund (:jlund)

Comment 130

•

10 years ago

Comment on attachment 8614348 [details] [diff] [review] [mozharness] Core changes: clean up dev config, add VCSToolsScript for support of hgtool and gittool on developer mode, create PlatfromMixin, plus create dir for hgtool/gittool Review of attachment 8614348 [details] [diff] [review]: ----------------------------------------------------------------- looks good. I'm mainly concerned with the which() changed behaviour. Have you done a full try run and checked the results? ::: mozharness/base/script.py @@ +529,4 @@ > return program > else: > + # If the exe file is defined in the configs let's use that > + exe = self.query_exe(program) I *think* this is okay. it scares me that this might cause spurious results if we define say 'python' in c['exes'] but use which('python') and query_exe('python') separately in the same script. But we should smoke out those instances anyway as I like your approach. ::: mozharness/mozilla/vcstools.py @@ +16,5 @@ > + > +VCS_TOOLS = ('hgtool.py', 'gittool.py') > + > + > +class VCSToolsScript(VCSScript): where is this class going to be used? @@ +20,5 @@ > +class VCSToolsScript(VCSScript): > + ''' This script allows us to fetch hgtool.py and gittool.py if > + we're running the script on developer mode. > + ''' > + @PreScriptAction('checkout') which action is this for? all I can see is checkout_* stuff: http://mxr.mozilla.org/build/search?string=def+checkout&find=mozharness&findi=&filter=%5E%5B%5E%5C0%5D*%24&hitlimit=&tree=build I'm guessing this checkout and VCSToolScript is only being used for your upcoming mh script

Attachment #8614348 - Flags: review?(jlund) → review+

Armen [:armenzg]

Assignee

Comment 131

•

10 years ago

Comment on attachment 8614348 [details] [diff] [review] [mozharness] Core changes: clean up dev config, add VCSToolsScript for support of hgtool and gittool on developer mode, create PlatfromMixin, plus create dir for hgtool/gittool Review of attachment 8614348 [details] [diff] [review]: ----------------------------------------------------------------- ::: mozharness/base/script.py @@ +529,4 @@ > return program > else: > + # If the exe file is defined in the configs let's use that > + exe = self.query_exe(program) In my eyes, when we set up a path to a binary on 'exes' it should mean the same as making the basedir of it as part of PATH. This is what this path accomplishes. With this change: which(program) inspects PATH and 'exes' query_exe(program) only inspects 'exes' I will test it on try. ::: mozharness/mozilla/vcstools.py @@ +16,5 @@ > + > +VCS_TOOLS = ('hgtool.py', 'gittool.py') > + > + > +class VCSToolsScript(VCSScript): The patch that Ben reviewed. @@ +20,5 @@ > +class VCSToolsScript(VCSScript): > + ''' This script allows us to fetch hgtool.py and gittool.py if > + we're running the script on developer mode. > + ''' > + @PreScriptAction('checkout') Correct.

Armen [:armenzg]

Assignee

Comment 132

•

10 years ago

https://treeherder.mozilla.org/#/jobs?repo=try&revision=eeda604378cb

Armen [:armenzg]

Assignee

Comment 133

•

10 years ago

Attached patch wip - custom.diff (obsolete) — Details — Splinter Review

Hi Ben, I still have to test this locally. I just want to know if it is the place where you foresee the hacking should take place and if the following testing plan makes sense to you. I still have to pass the tools tag to the job. I'm thinking of setting up my linux64 loaner as a staging buildbot-master (and a slave) and make the Windows and Mac machines to be slaves that connect to it. I'm mainly interested on seeing a chunk of each to run correctly for mozilla-beta and mozilla-release. Does this sound a good plan to you? I was thinking of handcrafting the buildbot.tac of the slaves. Is there a way to prevent loaner machines from talking to slavealloc and blow my handcrafted buildbot.tac files.

Attachment #8614910 - Flags: feedback?(bhearsum)

bhearsum@mozilla.com (:bhearsum)

Comment 134

•

10 years ago

Comment on attachment 8614910 [details] [diff] [review] wip - custom.diff Review of attachment 8614910 [details] [diff] [review]: ----------------------------------------------------------------- This is on the right track for sure, and I think your plan of attack sounds fine. I think you can make the slave do what you want by running "buildslave" directly, though I'm far from an expert these days. I think you're on a way out of date version of buildbocustom...eg, "post_update_builders" hasn't existed since mid-April. ::: process/release.py @@ +1343,5 @@ > + 'slavenames': branchConfig['platforms'][platform]['slaves'], > + 'category': builderPrefix(''), > + 'builddir': builddir, > + 'slavebuilddir': normalizeName(builddir, releaseConfig['productName']), > + 'factory': uv_factory, Typo here, this should be ui_uv_factory. @@ +1359,5 @@ > + 'event_group': 'update_verify', > + }, > + ''' > + }) > + post_update_builders.append(builder_name) One implication of putting these builders in the same data structure as the existing update verify is that they end up as blockers for the "ready for release" e-mail. Originally we talked about making sure these do not block automation when they're first turned on, and I think that's probably still something strive for. You'll probably want a new list for the ui update verify builders, and you'll probably need to make sure they end in the updates_done scheduler (https://github.com/mozilla/build-buildbotcustom/blob/master/process/release.py#L1854). https://github.com/bhearsum/buildbot-scheduler-graph can be very helpful in verifying scheduling changes.

Attachment #8614910 - Flags: feedback?(bhearsum) → feedback+

Pulsebot

Comment 135

•

10 years ago

https://hg.mozilla.org/integration/fx-team/rev/54511717179d

Armen [:armenzg]

Assignee

Comment 136

•

10 years ago

http://hg.mozilla.org/build/mozharness/rev/42ad3e4d3b1d Based on try push: https://treeherder.mozilla.org/#/jobs?repo=try&revision=eeda604378cb

Henrik Skupin [:whimboo][⌚️UTC+2]

Comment 137

•

10 years ago

Armen, I still miss the specification of the target version and target build id for the firefox-ui-update script. You should really add those two options!

Armen [:armenzg]

Assignee

•

10 years ago

https://hg.mozilla.org/mozilla-central/rev/54511717179d

Status: ASSIGNED → RESOLVED

Closed: 10 years ago

status-firefox41: --- → fixed

Resolution: --- → FIXED

Armen [:armenzg]

Assignee

Comment 142

•

10 years ago

Attached patch adding release builders (obsolete) — Details — Splinter Review

The patch passes and the patch adds the release builders (even comm-central) bhearsum: can you please have a look and have a try with your tool? I tried but it generated nothing: $ buildbot-scheduler-graph master.cfg ~/moz/patches/post_changes I'm also unsure if what i'm doing in this block is actually right: https://github.com/mozilla/build-buildbotcustom/blob/master/process/release.py#L1854

Attachment #8614910 - Attachment is obsolete: true

Attachment #8615594 - Attachment is obsolete: true

Attachment #8616167 - Flags: feedback?(bhearsum)

Armen [:armenzg]

Assignee

Updated

•

10 years ago

Status: RESOLVED → REOPENED

Resolution: FIXED → ---

bhearsum@mozilla.com (:bhearsum)

Comment 143

•

10 years ago

Comment on attachment 8616167 [details] [diff] [review] adding release builders Review of attachment 8616167 [details] [diff] [review]: ----------------------------------------------------------------- ::: process/release.py @@ +1913,5 @@ > + name=builderPrefix('%s_ui_updates_done' % channel), > + branch=sourceRepoInfo['path'], > + upstreamBuilders=[builderPrefix('%s_%s_ui_updates' % (releaseConfig['productName'], channel))], > + builderNames=ui_update_tests_builders[channel], > + )) You've got some copy/paste failure here. "ui_updates_done" doesn't make sense as a Scheduler name when this scheduler is _triggering_ ui update tests. The above one is called "updates_done" because it fires after updates complete. Something like "ui_update_verify" would probably make more sense. More importantly, this patch doesn't end up triggering ui update tests because the upstreamBuilders also suffer from copy/paste fail. You need to be downstream of the updates builder, so you probably need the exact same upstreamBuilders list as the above scheduler. r=me with this stuff fixed. I'd like to give some folks a heads up before it lands though, because it will generate new release mail that could panic people if it causes failures.

Attachment #8616167 - Flags: feedback?(bhearsum) → feedback+

Armen [:armenzg]

Assignee

Comment 144

•

10 years ago

Attached patch adding release builders (obsolete) — Details — Splinter Review

Minor changes: * Tackled your feedback * Do not add TB and esr31 builders * swapped channel & platform variables when creating the builder names to match similar builders

Attachment #8616167 - Attachment is obsolete: true

Attachment #8616711 - Flags: review?(bhearsum)

bhearsum@mozilla.com (:bhearsum)

Comment 145

•

10 years ago

Comment on attachment 8616711 [details] [diff] [review] adding release builders Review of attachment 8616711 [details] [diff] [review]: ----------------------------------------------------------------- ::: process/release.py @@ +1381,5 @@ > }, > }) > update_verify_builders[channel].append(builderName) > > + if releaseConfig['productName'] == 'firefox' and releaseConfig['version'] >= '38': Please add an actual switch for this, hardcoding is not the right solution. You'll need to update release config templates in buildbot-configs/mozilla. Everything else looks fine.

Attachment #8616711 - Flags: review?(bhearsum) → review-

Armen [:armenzg]

Assignee

Comment 146

•

10 years ago

Attached file differences.txt — Details

Attachment #8620588 - Flags: feedback?(bhearsum)

Armen [:armenzg]

Assignee

Comment 147

•

10 years ago

Attached patch [buildbotcustom] Add 'ui_update_tests' flag — Details — Splinter Review

Attachment #8620591 - Flags: review?(bhearsum)

Armen [:armenzg]

Assignee

Comment 148

•

10 years ago

Attached patch [buildbotcustom] add firefox ui update verify builders if 'ui_update_tests' is defined — Details — Splinter Review

Attachment #8616711 - Attachment is obsolete: true

Attachment #8620593 - Flags: review?(bhearsum)

bhearsum@mozilla.com (:bhearsum)

Comment 149

•

10 years ago

Comment on attachment 8620591 [details] [diff] [review] [buildbotcustom] Add 'ui_update_tests' flag Review of attachment 8620591 [details] [diff] [review]: ----------------------------------------------------------------- You need to update the Thunderbird release configs too. ::: mozilla/release-firefox-mozilla-esr38.py @@ +35,5 @@ > 'baseTag': 'FIREFOX_38_0esr', > }, > > } > +releaseConfig['ui_update_tests'] = True Are you sure you want to run this from the start on esr38 and mozilla-release? Maybe it would be better to just run it on Beta until we know it's working well. I don't feel strongly about this, just throwing it out there.

Attachment #8620591 - Flags: review?(bhearsum) → feedback+

bhearsum@mozilla.com (:bhearsum)

Updated

•

10 years ago

Attachment #8620593 - Flags: review?(bhearsum) → review+

bhearsum@mozilla.com (:bhearsum)

Updated

•

10 years ago

Attachment #8620588 - Flags: feedback?(bhearsum)

bhearsum@mozilla.com (:bhearsum)

Comment 150

•

10 years ago

(In reply to Ben Hearsum [:bhearsum] from comment #149) > Comment on attachment 8620591 [details] [diff] [review] > Add 'ui_update_tests' flag > > Review of attachment 8620591 [details] [diff] [review]: > ----------------------------------------------------------------- > > You need to update the Thunderbird release configs too. Per IRC, forget about this - I had a brainfart and forgot that the buildbotcustom parts default to off if the flag isn't present.

bhearsum@mozilla.com (:bhearsum)

Comment 151

•

10 years ago

Comment on attachment 8620591 [details] [diff] [review] [buildbotcustom] Add 'ui_update_tests' flag Per IRC, r=me if you flip this off for mozilla-release and esr38 for now.

Attachment #8620591 - Flags: review+

Armen [:armenzg]

Assignee

Comment 152

•

10 years ago

http://hg.mozilla.org/build/buildbot-configs/rev/4dbed1234656 http://hg.mozilla.org/build/buildbotcustom/rev/539e4c4be9d6 Sent to release-drivers: ######################## Hello all, We have landed the code to enable the Marionette Firefox UI update tests. These tests check updates through the browser's UI. We will initially roll this to mozilla-beta and evaluate that it runs smooth. They will be testing all locales for all betas starting from Gecko 38. The results will be showing up in the release automation notifications mailing list [1]. For now, please ignore the notifications if they don't run all the way to success. Work developed in bug 1148546 [2]. regards, Armen [1] https://groups.google.com/a/mozilla.com/forum/?hl=en#!forum/release-automation-notifications [2] https://bugzilla.mozilla.org/show_bug.cgi?id=1148546

Armen [:armenzg]

Assignee

Updated

•

10 years ago

Depends on: 1173933

Armen [:armenzg]

Assignee

Comment 153

•

10 years ago

Attached patch use the right gecko log path (obsolete) — Details — Splinter Review

Attachment #8621179 - Flags: review?(bhearsum)

Armen [:armenzg]

Assignee

Comment 154

•

10 years ago

Attached patch [buildbotcustom] use the right tools tag — Details — Splinter Review

Attachment #8621186 - Flags: review?(bhearsum)

bhearsum@mozilla.com (:bhearsum)

Updated

•

10 years ago

Attachment #8621179 - Flags: review?(bhearsum) → review+

bhearsum@mozilla.com (:bhearsum)

Updated

•

10 years ago

Attachment #8621186 - Flags: review?(bhearsum) → review+

Armen [:armenzg]

Assignee

Comment 155

•

10 years ago

Attached patch [mozharness] use the right gecko log path - do not foce --firefox-ui-branch unless we're checking out — Details — Splinter Review

Attachment #8621179 - Attachment is obsolete: true

Attachment #8621196 - Flags: review?(bhearsum)

Armen [:armenzg]

Assignee

Updated

•

10 years ago

Blocks: 1173976

Armen [:armenzg]

Assignee

Comment 156

•

10 years ago

I've reviewed and updated the documenation for this project: https://wiki.mozilla.org/Auto-tools/Projects/Marionette_update_tests I will keep this bug open for: * bug 1173933 to be fixed * see if there are any issues in the next few days with the jobs for the betas * enable mozilla-release and mozilla-esr38 if there are no issues * close this bug if there are no more issues Here are some matters that will need to be followed up on. #1) I assume that after uplifts or after dot releases are released new issues will pop up. We will deal with these once they happen. #2) We currently test all locales for all releases since the first Gecko 38 version. This will quickly add up and make run times of the jobs too long to wait for. This will be very noticeable in the beta channel since we have many many betas. This will need to be trimmed up in bug 1173976. After talking with whimboo and based on comment 79, these are some aspects of the project that will need to be followed up on in the future. #3) When we test updates, we simply test that we updated and not that we've updated to the right build. Passing the extra parameter of --update-target-buildid would take care of it. We can probably simply do something like this in here: [1] - retcode = self._run_test(installer_path, self.channel) + retcode = self._run_test( + installer_path=installer_path, + update_channel=self.channel, + build_id=rel_info['build_id']) We came to the conclusion that --update-target-version is not really necessary as mentioned on comment 79. #4) I will follow up in a new comment wrt to the second part of comment 79. [1] http://hg.mozilla.org/build/mozharness/file/ecd1d24223d3/scripts/firefox_ui_updates.py#l315

Armen [:armenzg]

Assignee

Comment 157

•

10 years ago

whimboo, in comment 79 you said the following: > 2. A beta user got updated to a RC build and has to be brought back to a > beta build. Therefore you need --update-allow-mar-channel > > Lets say a beta user was on 38.0b9 and got updated to 38.0RC. This RC build > is on the release channel, but beta users have to be brought back to beta > channel. Given that we currently cannot test updates in multiple steps vXb9 > -> RC -> v(X+1)b1, we have to run vXb9 -> RC, and RC -> v(X+1)b1 separately. > Using the RC build as source build only has the release channel active. The > option above can also enable the beta channel via update-settings.ini. > > ./firefox-ui-update --installer %Firefox38.0% --update-target-version=39.0b1 > --update-target-buildid=%whateveritwillbe% > --update-allow-mar-channel=firefox-mozilla-beta After speaking with lmandel, beta users are *not* switched to the release channel. Internally, they're still using mozilla-beta as their update channel. They simply are offered an RC build. Once they're not needed to test the RC anymore and there is a beta1, they will be updated to it without channing any channels. I've tested this and I did not need --update-allow-mar-channel. I can't test updating from beta to RC since we're not in that window at the moment but it shold simply work since we always specify --update-channel. This is what I've written in the documentation, let me know in which occassion would we need to use --update-allow-mar-channel: https://wiki.mozilla.org/Auto-tools/Projects/Marionette_update_tests#Beta_users_on_RC ## Beta users on RC ## When we're creating the RC builds, we update *beta* users to it (even though an RC, at the end, is meant for the release users). This means that after we're done with the RC, we get them back into the betas rather than leave them with the release users. Here's an example on how to test the update path for a *beta* user on an RC: python scripts/firefox_ui_updates.py \ --installer-url http://ftp.mozilla.org/pub/mozilla.org/firefox/candidates/38.0-candidates/build3/mac/en-US/Firefox%2038.0.dmg \ --firefox-ui-branch mozilla-beta \ --update-channel beta-localtest \ --cfg developer_config.py If I didn't set --update-channel, it would update to 38.0.5 instead of the latest beta on 39.

Flags: needinfo?(hskupin)

Henrik Skupin [:whimboo][⌚️UTC+2]

Comment 158

•

10 years ago

(In reply to Armen Zambrano G. (:armenzg - Toronto) from comment #156) > #2) We currently test all locales for all releases since the first Gecko 38 > version. > This will quickly add up and make run times of the jobs too long to wait for. > This will be very noticeable in the beta channel since we have many many > betas. > This will need to be trimmed up in bug 1173976. Too long to wait for? Kairo is running updates for all locales afaik across platforms and currently it takes approx. 20 min to complete. Keep in mind that we have a way smaller amount of available machines to run the tests on. Also as discussed in comment 92 we thought its clear that we do not want to reduce the locales. But lets this discuss on the newly filed bug. (In reply to Armen Zambrano G. (:armenzg - Toronto) from comment #157) > After speaking with lmandel, beta users are *not* switched to the release > channel. That's not what I ever told you. I said that we cannot run updates in multiple steps. So being able to test that beta users on the RC build are getting updated to N+1.b1 we have to allow the beta mar channel. Otherwise updates won't work. > Internally, they're still using mozilla-beta as their update channel. > They simply are offered an RC build. This is only the case when they had a beta build before, but not when you install a RC build. This will only be on the release channel! > ## Beta users on RC ## > When we're creating the RC builds, we update *beta* users to it (even though > an RC, at the end, is meant for the release users). > > This means that after we're done with the RC, we get them back into the > betas rather than leave them with the release users. Documentation is fine but see above for the limitation of the tests. > Here's an example on how to test the update path for a *beta* user on an RC: > python scripts/firefox_ui_updates.py \ > --installer-url > http://ftp.mozilla.org/pub/mozilla.org/firefox/candidates/38.0-candidates/ > build3/mac/en-US/Firefox%2038.0.dmg \ > --firefox-ui-branch mozilla-beta \ > --update-channel beta-localtest \ > --cfg developer_config.py > > If I didn't set --update-channel, it would update to 38.0.5 instead of the > latest beta on 39. And as I also told you the mar channel restriction might still only be in place on Windows but not Linux and Mac. So that is the reason why it works for you without having to allow the beta channel. I was looking now for the specific bug and here is what I trapped into originally similar to you now. Read through bug 1069977 to get more details. As bug 973933 shows cross-platform mar signing will be available as earliest as Firefox 40. That means starting with that version you definitely need the extra CLI option for all platforms.

Flags: needinfo?(hskupin)

bhearsum@mozilla.com (:bhearsum)

Comment 159

•

10 years ago

(In reply to Armen Zambrano G. (:armenzg - Toronto) from comment #157) > I've tested this and I did not need --update-allow-mar-channel. > I can't test updating from beta to RC since we're not in that window at the > moment but it shold simply work since we always specify --update-channel. We can set up a special channel for you if you want to test this now, it's pretty easy.

bhearsum@mozilla.com (:bhearsum)

Updated

•

10 years ago

Attachment #8621196 - Flags: review?(bhearsum) → review+

Robert Kaiser

•

10 years ago

Blocks: 1174174

Nick Thomas [:nthomas] (UTC+12)

Comment 164

•

10 years ago

All the jobs ran and failed for 39.0b6 build1. The problem was trying to update to the tag FIREFOX_39_0b6_RELEASE_RUNTIME in mozharness, but that only exists on tools. eg hg update -C -r FIREFOX_39_0b6_RELEASE_RUNTIME in dir /builds/slave/rel-m-beta-lx_beta_u_t_1-00000/scripts (timeout 1200 secs) ... abort: unknown revision 'FIREFOX_39_0b6_RELEASE_RUNTIME'! program finished with exit code 255

Nick Thomas [:nthomas] (UTC+12)

Comment 165

•

10 years ago

Attached patch [buildbotcustom] Tag fix — Details — Splinter Review

I *suspect* this is what you need to fix the jobs up in production - pull mozharness on FIREFOX_36_0b6_RELEASE, and tools with FIREFOX_39_0b6_RELEASE_RUNTIME. But I won't claim to have read the bug to see all the context.

Armen [:armenzg]

Assignee

Comment 166

•

10 years ago

Comment on attachment 8622859 [details] [diff] [review] [buildbotcustom] Tag fix Thanks Nick! Yes, for tools we want runtime (configs get bumped + tagged) [1] [1] http://hg.mozilla.org/build/tools/rev/8b5257cc071d

Attachment #8622859 - Flags: review?(bhearsum)

bhearsum@mozilla.com (:bhearsum)

Updated

•

10 years ago

Attachment #8622859 - Flags: review?(bhearsum) → review+

Armen [:armenzg]

Assignee

Comment 167

•

10 years ago

https://hg.mozilla.org/build/buildbotcustom/rev/7d256441b0f5

•

10 years ago

Attached patch [mozharness] Prevent running the harness without explicitely setting the --tools-tag — Details — Splinter Review

•

10 years ago

Thanks Henrik! I will look into specifying the symbols. After speaking with chmanchester we will add --gecko-log - . We will see if we get more info. I also have to fix the issue the 32-bit & 64-bit jobs are using the same update verify configs. * Crash - bug 1176358 * Windows checkouts - bug 1176359 * Differentiate 32-bit and 64-bit Linux/Windows platforms - bug 1176360

Depends on: 1176358, 1176359, 1176360

Armen [:armenzg]

Assignee

Comment 176

•

10 years ago

The only bug left in here is bug 1176358, which we're going to investigate. I will be closing this bug and return the Windows and Mac loaners. If there are any other issues please file a Release Engineering General Automation bug and request when it could be prioritized.

Armen [:armenzg]

Assignee

Updated

•

10 years ago

Status: REOPENED → RESOLVED

Closed: 10 years ago → 10 years ago

Resolution: --- → FIXED

Armen [:armenzg]

Assignee

Updated

•

10 years ago

Blocks: 1181682

Henrik Skupin [:whimboo][⌚️UTC+2]

Comment 177

•

10 years ago

(In reply to Armen Zambrano Gasparnian [:armenzg] from comment #176) > The only bug left in here is bug 1176358, which we're going to investigate. > I will be closing this bug and return the Windows and Mac loaners. > If there are any other issues please file a Release Engineering General > Automation bug and request when it could be prioritized. What about the other remaining issues we talked about late in June and which you wanted to work on in July? This was e.g. using the target-build-id for stronger checks, and others. I don't have the list handy, so it would be great if you can file a new bug for it.

Flags: needinfo?(armenzg)

Armen [:armenzg]

Assignee

Comment 178

•

10 years ago

Hi whimboo, I don't remember the list either. I believe we deemed it as not blockers. Comment 79 and commnet 156 probably have some of the info you refer to. I'm not planning on working on adding those.

Flags: needinfo?(armenzg)

Henrik Skupin [:whimboo][⌚️UTC+2]

Comment 179

•

10 years ago

(In reply to Armen Zambrano Gasparnian [:armenzg] from comment #178) > Hi whimboo, I don't remember the list either. I believe we deemed it as not > blockers. Those are blockers given that with missing target build id checks the update tests totally miss to check if the update was successful for the expected candidate/release. And without the mar channel handling, we will fail testing release builds to the next beta build. > Comment 79 and commnet 156 probably have some of the info you refer to. > I'm not planning on working on adding those. Comment 79 seems to cover it well enough. I'm not happy in seeing that we leave this features out because it causes a drastic limitation of our update tests. I will leave it up to Kairo to repond and tell us his own opinion. When he agrees we have to find someone to get it implemented.

Flags: needinfo?(kairo)

Armen [:armenzg]

Assignee

Comment 180

•

10 years ago

Please file a new bug for this and mention it on this bug the new bug. It's hard to use this bug anymore (due to the 150+ comments). Thanks.

Robert Kaiser

Comment 181

•

10 years ago

I agree with Henrik. Please CC me on the new bug, we'll need those things before we can move away from running mozmill tests.

Flags: needinfo?(kairo)

Henrik Skupin [:whimboo][⌚️UTC+2]

Comment 182

•

10 years ago

For the target-build-id checks we already have bug 1174174. I will check if the other bugs might also have been created already before filing new ones.

Component: General Automation → Release Automation

QA Contact: catlee → bhearsum

Henrik Skupin [:whimboo][⌚️UTC+2]

Updated

•

10 years ago

Blocks: 1191893

Henrik Skupin [:whimboo][⌚️UTC+2]

Updated

•

10 years ago

Blocks: 1191896

Henrik Skupin [:whimboo][⌚️UTC+2]

Updated

•

10 years ago

No longer blocks: 1191896

Armen [:armenzg]

Assignee

Comment 183

•

10 years ago

Attached patch [buildbotcustom] back out all changes required for running Firefox UI tests on builders — Details — Splinter Review

This removes any changes which we landed on buildbotcustom related to Firefox UI tests. Since we're moving to testers (bug 1192309), we are likely not to need this code. I will ask for a review when we're sure.

Armen [:armenzg]

Assignee

Updated

•

10 years ago

No longer blocks: 1159527

Armen [:armenzg]

Assignee

Comment 184

•

10 years ago

Let's only block bug 1182796 (the tracking bug) to have a one stop bug.

Blocks: 1182796
No longer blocks: 1173976, 1174174, 1191893, 1181682

run.txt 10 years ago Armen [:armenzg] 38.61 KB, text/plain		Details
good run 10 years ago Armen [:armenzg] 6.86 KB, text/plain		Details
errors.txt 10 years ago Armen [:armenzg] 39.49 KB, text/plain		Details
fullrun.txt 10 years ago Armen [:armenzg] 861.94 KB, text/plain		Details
fullrun.txt 10 years ago Armen [:armenzg] 752.00 KB, text/plain	bhearsum : feedback+	Details
add marionette as an option to verify.sh 10 years ago Armen [:armenzg] 4.58 KB, patch	bhearsum : feedback+	Details \| Diff \| Splinter Review
fullrun.txt 10 years ago Armen [:armenzg] 120.00 KB, text/plain		Details
tools.diff 10 years ago Armen [:armenzg] 7.61 KB, patch	bhearsum : feedback+	Details \| Diff \| Splinter Review
remove.diff 10 years ago Armen [:armenzg] 876 bytes, patch	bhearsum : feedback+	Details \| Diff \| Splinter Review
"complete" mode output 10 years ago Armen [:armenzg] 97.47 KB, text/plain		Details
fullrun.txt 10 years ago Armen [:armenzg] 780.60 KB, text/plain		Details
bump firefox-ui-tests versions used (for tracking purposes) 10 years ago Armen [:armenzg] 430 bytes, patch		Details \| Diff \| Splinter Review
[tools] Add marionette update testing mode 10 years ago Armen [:armenzg] 8.20 KB, patch	bhearsum : review-	Details \| Diff \| Splinter Review
Use mozinstall 1.12 for firefox-ui-tests 10 years ago Armen [:armenzg] 52 bytes, text/x-github-pull-request	chmanchester : review+	Details \| Review
[tools] Add marionette update testing mode 10 years ago Armen [:armenzg] 9.66 KB, patch		Details \| Diff \| Splinter Review
[tools] Add marionette update testing mode 10 years ago Armen [:armenzg] 9.75 KB, patch	bhearsum : review-	Details \| Diff \| Splinter Review
[tools] Add marionette update testing mode 10 years ago Armen [:armenzg] 11.40 KB, patch		Details \| Diff \| Splinter Review
Create firefox UI updates script 10 years ago Armen [:armenzg] 17.60 KB, patch	bhearsum : feedback+	Details \| Diff \| Splinter Review
Core mozharness changes 10 years ago Armen [:armenzg] 5.28 KB, patch		Details \| Diff \| Splinter Review
fx_mozharness.diff 10 years ago Armen [:armenzg] 13.95 KB, patch		Details \| Diff \| Splinter Review
fx_mozharness.diff 10 years ago Armen [:armenzg] 15.10 KB, patch	bhearsum : feedback+	Details \| Diff \| Splinter Review
Block using UpdateVerifyConfig 10 years ago Armen [:armenzg] 1.10 KB, text/plain	bhearsum : review+	Details
[wip] self-discerning config 10 years ago Armen [:armenzg] 91 bytes, patch	jgriffin : feedback+ jlund : feedback+	Details \| Diff \| Splinter Review
2nd test fails and the browser is not killed 10 years ago Armen [:armenzg] 1.41 MB, text/plain		Details
[mozharness] Core changes: clean up dev config, add VCSToolsScript for support of hgtool and gittool on developer mode, create PlatfromMixin 10 years ago Armen [:armenzg] 7.84 KB, patch		Details \| Diff \| Splinter Review
fx_mozharness.diff 10 years ago Armen [:armenzg] 18.95 KB, patch		Details \| Diff \| Splinter Review
[mozharness] Core changes: clean up dev config, add VCSToolsScript for support of hgtool and gittool on developer mode, create PlatfromMixin, plus create dir for hgtool/gittool 10 years ago Armen [:armenzg] 9.64 KB, patch	jlund : review+	Details \| Diff \| Splinter Review
[mozharness] Firefox UI update tests 10 years ago Armen [:armenzg] 19.08 KB, patch	bhearsum : review+	Details \| Diff \| Splinter Review
wip - custom.diff 10 years ago Armen [:armenzg] 5.93 KB, patch	bhearsum : feedback+	Details \| Diff \| Splinter Review
wip - custom.diff 10 years ago Armen [:armenzg] 7.22 KB, patch		Details \| Diff \| Splinter Review
adding release builders 10 years ago Armen [:armenzg] 8.53 KB, patch	bhearsum : feedback+	Details \| Diff \| Splinter Review
adding release builders 10 years ago Armen [:armenzg] 8.77 KB, patch	bhearsum : review-	Details \| Diff \| Splinter Review
differences.txt 10 years ago Armen [:armenzg] 12.73 KB, text/plain		Details
[buildbotcustom] Add 'ui_update_tests' flag 10 years ago Armen [:armenzg] 6.04 KB, patch	bhearsum : review+ bhearsum : feedback+	Details \| Diff \| Splinter Review
[buildbotcustom] add firefox ui update verify builders if 'ui_update_tests' is defined 10 years ago Armen [:armenzg] 8.73 KB, patch	bhearsum : review+	Details \| Diff \| Splinter Review
use the right gecko log path 10 years ago Armen [:armenzg] 2.08 KB, patch	bhearsum : review+	Details \| Diff \| Splinter Review
[buildbotcustom] use the right tools tag 10 years ago Armen [:armenzg] 1.11 KB, patch	bhearsum : review+	Details \| Diff \| Splinter Review
[mozharness] use the right gecko log path - do not foce --firefox-ui-branch unless we're checking out 10 years ago Armen [:armenzg] 4.17 KB, patch	bhearsum : review+	Details \| Diff \| Splinter Review
[buildbotcustom] Tag fix 10 years ago Nick Thomas [:nthomas] (UTC+12) 2.33 KB, patch	bhearsum : review+	Details \| Diff \| Splinter Review
[buildbotcustom] fix double list + set python interpreter 10 years ago Armen [:armenzg] 2.02 KB, patch	armenzg : review+	Details \| Diff \| Splinter Review
[buildbotcustom] Pass arg properly 10 years ago Armen [:armenzg] 1.20 KB, patch	armenzg : review+	Details \| Diff \| Splinter Review
[mozharness] Prevent running the harness without explicitely setting the --tools-tag 10 years ago Armen [:armenzg] 5.02 KB, patch		Details \| Diff \| Splinter Review
[mozharness] Prevent running the harness without explicitely setting the --tools-tag 10 years ago Armen [:armenzg] 5.06 KB, patch	bhearsum : review+	Details \| Diff \| Splinter Review
[buildbotcustom] back out all changes required for running Firefox UI tests on builders 10 years ago Armen [:armenzg] 9.23 KB, patch		Details \| Diff \| Splinter Review