Use more descriptive assertion messages for update tests (e.g. "Update has been found" vs. "AssertionError")

RESOLVED FIXED in Firefox 41

Status

defect
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: whimboo, Assigned: whimboo)

Tracking

42 Branch
mozilla44
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox41 fixed, firefox42 fixed, firefox43 fixed, firefox44 fixed, firefox-esr38 fixed)

Details

Attachments

(1 attachment)

The failure details are not satisfying given that we do not report any details about that failure. Reason is that we do a simple assert:

> assert self.deck.selected_panel == self.deck.check_for_updates
AssertionError

We should better use the unittest type assertions if possible instead.
So far the failure only happened with Firefox 42 and other locales than en-US and Windows 8 x64. Something I would propose is to fix the assertion output for all branches and then try to figure out what's causing this problem if it happens more than that.
OS: Unspecified → Windows 8
Hardware: Unspecified → x86_64
Summary: Intermittent failure in test_direct_update.py / test_fallback_update.py: AssertionError → Intermittent failure in test_direct_update.py / test_fallback_update.py: AssertionError (self.deck.selected_panel == self.deck.check_for_updates)
Looking more into one of the build results shows me that the direct update test which runs first does not even check for updates but directly starts to download it:

00:01:10.156 *** AUS:SVC readStatusFile - status: downloading, path: C:\Users\mozauto\AppData\Local\Mozilla\Firefox\firefox\updates\0\update.status

As you can see in the path the build is getting installed into a shared location. So most likely a former build did not uninstall it and the update.status file survived. That means what we have to do for now is to make use of the workspace option, which i have added recently on bug 1199574. With that we can install Firefox into the Jenkins workspace which definetely gets cleaned up before each build.

To fix the workspace issue I filed: https://github.com/mozilla/mozmill-ci/issues/643.

Otherwise I cleaned-up the win-81-62-2 box from the remaining updates folder, so updates should work again now.
So the remaining issue are the better assertion descriptions, which are necessary to better see what's wrong with failing update tests. I will morph the summary and work on a fix to get it improved across all branches.

Armen, I believe this is also wanted by RelEng. They most likely don't want to check the logs all the time but have a crisp failure message. Maybe that fix might block the production use?
Assignee: nobody → hskupin
Status: NEW → ASSIGNED
OS: Windows 8 → All
Hardware: x86_64 → All
Summary: Intermittent failure in test_direct_update.py / test_fallback_update.py: AssertionError (self.deck.selected_panel == self.deck.check_for_updates) → Use more descriptive assertion messages for update tests (e.g. "Update has been found" vs. "AssertionError")
That would be nice.

I give them a summary of all jobs in here:
https://dxr.mozilla.org/mozilla-central/source/testing/mozharness/scripts/firefox_ui_tests/update_release.py?offset=500#318

Perhaps a file with an extended summary or even an html summary could be generated and uploaded as an artifact.
This is a scope creep.
Armen that's not what this bug is actually about. It's more about what actual assertions in the test use as description, so it's immediately visible what's broken.

So one example would be to not only see "AssertionError" as failure but "downloading != checkforupdates".
The underlying issue for this massive fallout was bug 1206617 which also let the update check stuck.

I will work on this bug so we can see better failure details on treeherder.
After talking to David on IRC the best would be to move all plain assert calls out of our libraries and if possible create helper methods on the appropriate test class. This would work fine for the update tests, which I mainly cover via this bug report.
This patch fixes the those assertions which didn't supply any details of what's being checked yet. Some messages I had to extend to also show the expected vs. actual values. That was necessary at least until we have better assertion message displaying for our tests. I filed bug 1207067 for some improvements.
Attachment #8664125 - Flags: review?(dburns)
Attachment #8664125 - Flags: review?(dburns) → review+
PR merged to master:
https://github.com/mozilla/firefox-ui-tests/commit/66c5753a057610df0bf1d5b8a9080ccf4e93e31c

I will watch the results for at least today before cherry-picking the patch for the other branches.
Target Milestone: --- → Firefox 44
Product: Mozilla QA → Testing
You need to log in before you can comment on or make changes to this bug.