881435 - navigator.id.get() needs a test page in gaia UI Tests app

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Gaia, defect)

Description

[Jed Parsons (use needinfo, please) [:jedp, :jparsons]]

Create a page like the one for 'navigator.mozId tests' in the gaia UI Tests app.  It can't use the existing navigator.mozId tests page because the watch and get apis don't mix.

Comment 1

[Jed Parsons (use needinfo, please) [:jedp, :jparsons]]

Attachment: gaia PR - add navigator.mozId.get tests to UI Tests app

Comment 2

[Jed Parsons (use needinfo, please) [:jedp, :jparsons]]

Comment on attachment 760666 [details]
gaia PR - add navigator.mozId.get tests to UI Tests app

Hi, Fernando,

Can I ask you to review this addition to the gaia UI Tests app?

It's very similar to the existing navigator.mozId tests; It just tests a different part of the Persona api.

Thanks!
j

Comment 3

[Fernando Jiménez Moreno [:ferjm]]

Comment on attachment 760666 [details]
gaia PR - add navigator.mozId.get tests to UI Tests app

Looks good to me.

Even if this patch didn't cause it, the travis run failed. It would be great to have a green travis run before merging this PR.

Comment 4

[Jed Parsons (use needinfo, please) [:jedp, :jparsons]]

(In reply to Fernando Jiménez Moreno [:ferjm] (needinfo instead of CC, please) from comment #3)
> Comment on attachment 760666 [details]
> gaia PR - add navigator.mozId.get tests to UI Tests app
> 
> Looks good to me.

Thanks for the speedy review, Fernando.

> Even if this patch didn't cause it, the travis run failed. It would be great
> to have a green travis run before merging this PR.

Yes, it blew up on "[costcontrol] Custom VIVO configuration Test Suite > Balance request matches the proper pattern".  I'll try to find someone to look into that.

Comment 5

[Jed Parsons (use needinfo, please) [:jedp, :jparsons]]

Talked with lightsofapollo on irc.  Sounds like this is a known intermittent failure.  He suggested we go ahead and merge this.

Which I have done.

Comment 6

[Jed Parsons (use needinfo, please) [:jedp, :jparsons]]

Hi, Triage!  It would be good to get this uplifted into gaia 1.1 so QA can run automated tests for this part of the browserid api on the nightly eng builds.

Risk is low; it's a test app that's not normally part of the build.
Benefit for all is increased automated test coverage, ergo stability.

Comment 7

[Jason Smith [:jsmith]]

(In reply to Jed Parsons [:jparsons] from comment #6)
> Hi, Triage!  It would be good to get this uplifted into gaia 1.1 so QA can
> run automated tests for this part of the browserid api on the nightly eng
> builds.
> 
> Risk is low; it's a test app that's not normally part of the build.
> Benefit for all is increased automated test coverage, ergo stability.

This reminds me that we really need to include the a=test only for Gaia type of approval. There's zero impact on these changes to core Gaia.

Comment 8

[John Ford [:jhford] CET/CEST Berlin Time]

I was not able to uplift this bug to v1-train.  If this bug has dependencies which are not marked in this bug, please comment on this bug.  If this bug depends on patches that aren't approved for v1-train, we need to re-evaluate the approval.  Otherwise, if this is just a merge conflict, you might be able to resolve it with:

  git checkout v1-train
  git cherry-pick -x -m1 278d090ceada5ac7ab0519116cd23a3f2be42267
  <RESOLVE MERGE CONFLICTS>
  git commit

Comment 9

[Jed Parsons (use needinfo, please) [:jedp, :jparsons]]

Hi, John,

Thanks.  I cherry-picked and made this PR for v1-train:

https://github.com/mozilla-b2g/gaia/pull/10585

(The only issue was that there are more test apps on master than v1-train, so the index.html file didn't merge nicely on v1-train.)

Does that look ok?

Comment 10

[John Ford [:jhford] CET/CEST Berlin Time]

(In reply to Jed Parsons [:jedp, :jparsons] from comment #9)
> Hi, John,
> 
> Thanks.  I cherry-picked and made this PR for v1-train:
> 
> https://github.com/mozilla-b2g/gaia/pull/10585
> 
> (The only issue was that there are more test apps on master than v1-train,
> so the index.html file didn't merge nicely on v1-train.)
> 
> Does that look ok?

I can't speak to the contents of the patch, but it looks reasonable to me.  I've merged it with

v1-train: a1560eb

Comment 11

[John Ford [:jhford] CET/CEST Berlin Time]

1.1hd: a1560eb8724536703e2fadfcdbd3883c0167ec70