Closed Bug 1057996 Opened 6 years ago Closed 6 years ago

[Messages][Tests] ThreadUI unit tests aren't run if full

Categories

(Firefox OS Graveyard :: Gaia::SMS, defect)

ARM
Gonk (Firefox OS)
defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: azasypkin, Assigned: azasypkin)

References

Details

(Whiteboard: [sms-sprint-2.1S3])

Attachments

(1 file)

Typo in patch for bug 1053968 caused exception on test suite parsing stage, so that the rest of tests weren't executed. Currently test agent just swallows such exceptions that makes it really hard to identify :(
Hey Steve,

Here is the patch that fixes typo and a few tests that were failing when typo was fixed.

Thanks!
Attachment #8478201 - Flags: review?(schung)
We still need to make something with test agent\mocha to avoid that in the future, will check if something similar is filed against it
Whiteboard: [sms-sprint-2.1S3]
(In reply to Oleg Zasypkin [:azasypkin] from comment #2)
> We still need to make something with test agent\mocha to avoid that in the
> future, will check if something similar is filed against it

Just some quick question before running the tests: Sorry that I could not find out the typo at first glance... why do we need to replace the assertion with verifying the attribute in this patch? Although verifying the attribute is fine for me, it still makes sense that verifying the L10n setAttributes call correctly.
The typo is here: https://github.com/mozilla-b2g/gaia/pull/23267/files#diff-d3f17b4905e6f17ad136aabbdc262db9L2823

Assert on the same level as "test" function and it throws on test parsing stage because "button" isn't available yet :)
(In reply to Steve Chung [:steveck] from comment #3)
> (In reply to Oleg Zasypkin [:azasypkin] from comment #2)
> Although verifying the attribute
> is fine for me, it still makes sense that verifying the L10n setAttributes
> call correctly.
Yeah, it's fine for me too, but I believe I changed it only for the cases where "setAttributes" wasn't called at all (download(-ing) button) and tests were invalid in fact, but they weren't executed due to exception thrown on "parsing" stage.
Comment on attachment 8478201 [details] [review]
GitHub pull request URL

Thanks for the clear explanation!
Just some small nit, so r=me
Attachment #8478201 - Flags: review?(schung) → review+
(In reply to Steve Chung [:steveck] from comment #6)
> Comment on attachment 8478201 [details] [review]
> GitHub pull request URL
> 
> Thanks for the clear explanation!
> Just some small nit, so r=me

Thanks for review! Nits fixed and landed.

Master: https://github.com/mozilla-b2g/gaia/commit/8b315117fe19b20bf8c3253a796aa1b5be14b47f
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
(In reply to Oleg Zasypkin [:azasypkin] from comment #2)
> We still need to make something with test agent\mocha to avoid that in the
> future, will check if something similar is filed against it

Bug 1036955 :)
(In reply to Julien Wajsberg [:julienw] (PTO 08/19 -> 09/15; contact schung for SMS matters) from comment #8)
> (In reply to Oleg Zasypkin [:azasypkin] from comment #2)
> > We still need to make something with test agent\mocha to avoid that in the
> > future, will check if something similar is filed against it
> 
> Bug 1036955 :)

Nice, nice, nice! :)
You need to log in before you can comment on or make changes to this bug.