Closed Bug 1109005 Opened 5 years ago Closed 5 years ago

Remove duplication from B2G test variable files

Categories

(Firefox OS Graveyard :: Gaia::UI Tests, defect)

defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: davehunt, Assigned: davehunt)

References

Details

Attachments

(1 file)

Now that bug 1089536 is resolved and we have a new Marionette client, we can remove the duplication from the test variable files. This will require updates to all Jenkins jobs once it's merged.
That's my kind of patch: 151 additions and 1,340 deletions
Attachment #8533669 - Flags: review?(zcampbell)
Once merged we'll need to add --testvars=/path/to/common.json to each job.
Blocks: 1108845
Attachment #8533669 - Flags: review?(zcampbell) → review?(jlorenzo)
Comment on attachment 8533669 [details] [review]
Link to Github pull-request: https://github.com/mozilla/webqa-credentials/pull/184

Waw, that was a tough one!

To do my review, I downloaded the raw diff that I opened in my editor. I made some search and replace to erase the common deleted part in order to highlight what was not commonly deleted (like empty mac addresses or useless "attached_to"). Also, I haven't found anything missed.
By the way, the indent modification in some files made the review harder.

I also checked the additions in common.json. Everything looks ok.

I checked that every device is in the PST timezone. They are all in MTV.

In the end, this patch looks good to me. I prefer to merge it tomorrow, in order to have time to catch any problem on Jenkins. I needinfo myself to do it.
Flags: needinfo?(jlorenzo)
Attachment #8533669 - Flags: review?(jlorenzo) → review+
(In reply to Johan Lorenzo [:jlorenzo] (QA) from comment #3)
> By the way, the indent modification in some files made the review harder.

You're right, that should have been a separate commit. Thanks for pointing it out.

> In the end, this patch looks good to me. I prefer to merge it tomorrow, in
> order to have time to catch any problem on Jenkins. I needinfo myself to do
> it.

As I mentioned on IRC yesterday, I took care of landing this, however it was necessary for me to back it out due to the likelihood that it will break v2.0 and v2.1 compatibility. Support for multiple test variable files landed in the Marionette runner only recently and has not been uplifted to these branches. As all Jenkins nodes share the same test variable files, we would need to support this new feature in the other branches.
Flags: needinfo?(jlorenzo)
Depends on: 1089536
Relanded now that bug 1089536 has been uplifted.
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.