642409 - Improve assertions module

Status: RESOLVED FIXED

(Mozilla QA Graveyard :: Mozmill Tests, defect)

Description

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

Given Clint's review over on bug 637880 some improvements have to be made to our assertions module.

Comment 1

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

Attachment: Patch v1

Comment 2

[Geo Mealer [:geo] -- This account is inactive after 2015-07-07]

Comment on attachment 519869 [details] [diff] [review]
Patch v1

r+. The code that's there looks fine, though I can't tell from your attachment comment precisely what you meant to change so can't speak to any high level.

Comment 3

[Heather Arthur [:harth]]

Comment on attachment 519869 [details] [diff] [review]
Patch v1


>+    if (!!diagnosis)
>+      message = !!message ? message + " - " + diagnosis : diagnosis;
> 
As discussed in irc, if(!!diagnosis) == if(diagnosis) in all cases. Same for the ternary op.

>     // Build result data
>-    let frame = findCallerFrame(this._startFrame);
>+    let frame = findCallerFrame(Components.stack);


I think Clint meant to take out the findCallerFrame() function. We do some stack parsing now
in bug 607450.
 

I have some of these changes in the new patch for adding assertions in bug 637880

Comment 4

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

(In reply to comment #3)
> >+    if (!!diagnosis)
> >+      message = !!message ? message + " - " + diagnosis : diagnosis;
> > 
> As discussed in irc, if(!!diagnosis) == if(diagnosis) in all cases. Same for
> the ternary op.

I will update this part.

> >     // Build result data
> >-    let frame = findCallerFrame(this._startFrame);
> >+    let frame = findCallerFrame(Components.stack);
> 
> I think Clint meant to take out the findCallerFrame() function. We do some
> stack parsing now in bug 607450.

He meant that for your work to port that over to Mozmill. We will be able to remove it once we use Mozmill 2.0.

Comment 5

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

I will wait with further work on that bug until bug 637880 has been fixed. So I can pull back all the updated code.

Comment 6

[cmtalbert]

Comment on attachment 519869 [details] [diff] [review]
Patch v1

Are you going to pull the throws/donotthrow stuff into this patch as well?

Comment 7

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

(In reply to comment #6)
> Are you going to pull the throws/donotthrow stuff into this patch as well?

No, that's part of bug 642081.

Comment 8

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

Attachment: Patch v2

Only small updates backported from the discussion on bug 637880.

Comment 9

[Geo Mealer [:geo] -- This account is inactive after 2015-07-07]

Comment on attachment 521983 [details] [diff] [review]
Patch v2

Looks fine, r+

Comment 10

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

Landed as:
https://github.com/geoelectric/mozmill-api-refactor/commit/7820a8523082cc0a766b7491d31c59d4c77d0a5f