Add TBPL support for Android instrumentation tests

RESOLVED FIXED

Status

RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: nalexander, Assigned: KWierso)

Tracking

(Depends on: 1 bug, Blocks: 1 bug)

Trunk
All
Android
Dependency tree / graph

Details

Attachments

(1 attachment)

My jobs appear to be running (\o/) and are currently listed as "U".  If it's easy, I'd like to make the instrumentation tests a suite like "I (Ba Br)" for Instrumentation -- Background, Browser but I'm not picky.  (We might want to keep single letters for the future.  Perhaps "Inst", or "Ai" for Android instrumentation).

This is the Android instrumentation analog to Bug 974358.
(Reporter)

Updated

4 years ago
Component: Testing → Tinderboxpushlog
Product: Firefox for Android → Webtools
(Reporter)

Comment 1

4 years ago
These jobs are not yet shown, so this is not urgent; but it's nicer to look at while developing the scripts.
(Reporter)

Comment 2

4 years ago
kwierso: the jobs are running on Ash and Cedar.  They were added to the buildbot configurations by Bug 1064010.
Depends on: 1064010, 1064012
Flags: needinfo?(kwierso)
(Assignee)

Comment 3

4 years ago
Created attachment 8489524 [details] [diff] [review]
1067480

I think this is what we want.
Attachment #8489524 - Flags: review?(emorley)
Flags: needinfo?(kwierso)
Attachment #8489524 - Attachment is patch: true
(Reporter)

Comment 4

4 years ago
Comment on attachment 8489524 [details] [diff] [review]
1067480

That seems reasonable.  Thanks for moving on this so quickly.
Attachment #8489524 - Flags: feedback+
(Assignee)

Updated

4 years ago
Assignee: nobody → kwierso

Comment 5

4 years ago
Comment on attachment 8489524 [details] [diff] [review]
1067480

Thank you for taking this on Wes :-)

The patch as written does label the jobs correctly. 

My only thoughts are:
1) The "B[x]" symbols tend to be for builds, not tests.
2) These jobs appear in a group, and with the attached patch that group appears in the middle of non-grouped jobs. I think we should try and move the group to after mochitest or something.

Asking for review from Ryan, since he's spent some time making the job names/groups/symbols consistent across the board, so I think he might have an opinion :-)
Attachment #8489524 - Flags: review?(emorley) → review?(ryanvm)
Comment on attachment 8489524 [details] [diff] [review]
1067480

Review of attachment 8489524 [details] [diff] [review]:
-----------------------------------------------------------------

Wes, if you and Nick are OK with the comments below, I've already made the changes below locally and rebased it on top of bug 1068764.

::: js/Config.js
@@ +287,5 @@
>                               "Unknown B2G Device Image Nightly",
>                               "Unknown B2G Device Image Nightly (Engineering)"],
>      "L10n Repack": ["L10n Nightly"],
>      "Android x86 Test Combos": ["Android x86 Test Set"],
> +    "Android Instrumentation Tests": ["Android Instrumentation Background", 

Trailing whitespace.

@@ +513,5 @@
>      "Marionette Framework Unit Tests" : "Mn",
>      "Marionette WebAPI Tests" : "Mnw",
>      "Android x86 Test Set" : "S",
>      "Android x86 Test Combos" : "Sets",
> +    "Android Instrumentation Tests" : "Inst",

I think we can get away with "I" here since it's just being used as a group symbol.

@@ +515,5 @@
>      "Android x86 Test Set" : "S",
>      "Android x86 Test Combos" : "Sets",
> +    "Android Instrumentation Tests" : "Inst",
> +    "Android Instrumentation Background" : "Ba",
> +    "Android Instrumentation Browser" : "Br",

What Ed said - these should come after the reftests. Otherwise, I think we're OK using Ba and Br here.
Attachment #8489524 - Flags: review?(ryanvm) → review+
(Reporter)

Comment 7

4 years ago
(In reply to Ryan VanderMeulen [:RyanVM UTC-4] from comment #6)
> Comment on attachment 8489524 [details] [diff] [review]
> 1067480
> 
> Review of attachment 8489524 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Wes, if you and Nick are OK with the comments below, I've already made the
> changes below locally and rebased it on top of bug 1068764.

Fine by me.
 
> ::: js/Config.js
> @@ +287,5 @@
> >                               "Unknown B2G Device Image Nightly",
> >                               "Unknown B2G Device Image Nightly (Engineering)"],
> >      "L10n Repack": ["L10n Nightly"],
> >      "Android x86 Test Combos": ["Android x86 Test Set"],
> > +    "Android Instrumentation Tests": ["Android Instrumentation Background", 
> 
> Trailing whitespace.
> 
> @@ +513,5 @@
> >      "Marionette Framework Unit Tests" : "Mn",
> >      "Marionette WebAPI Tests" : "Mnw",
> >      "Android x86 Test Set" : "S",
> >      "Android x86 Test Combos" : "Sets",
> > +    "Android Instrumentation Tests" : "Inst",
> 
> I think we can get away with "I" here since it's just being used as a group
> symbol.

OK; wasn't sure if we wanted to camp the high-level namespace.
(Assignee)

Comment 8

4 years ago
Fine with me.

Updated

4 years ago
Status: NEW → ASSIGNED

Updated

4 years ago
Blocks: 1067482
Ryan/Wes, what were your thoughts on:

(In reply to Ed Morley [:edmorley] from comment #5)
> 1) The "B[x]" symbols tend to be for builds, not tests.

(ie maybe using something that doesn't start with "B")
(In reply to Ed Morley [:edmorley] from comment #10)
> Ryan/Wes, what were your thoughts on:
> 
> (In reply to Ed Morley [:edmorley] from comment #5)
> > 1) The "B[x]" symbols tend to be for builds, not tests.
> 
> (ie maybe using something that doesn't start with "B")

(In reply to Ryan VanderMeulen [:RyanVM UTC-4] from comment #6)
> Otherwise, I think we're OK using Ba and Br here.

Given that they're inside the I() group rather than standalone jobs, I don't see it being an issue here.
Ah missed that :-)
(Reporter)

Comment 13

4 years ago
(In reply to Ed Morley [:edmorley] from comment #12)
> Ah missed that :-)

I'm not strongly disposed to any such choices, BTW.  I anticipated having multiple suites, but further exploration with the foopy/Panda set up suggests that we might want to fold them all together:

1) the time to configure a job on a Panda job is ~10 minutes.

2) the time to run a suite, even a large-ish one, is ~5 minutes.  A small-ish suite is ~1 minute.

Let's run them separately and reconsider after we have some more experience.  Thanks, folks!
Depends on: 1071160
In production :)
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Product: Webtools → Tree Management
Product: Tree Management → Tree Management Graveyard
You need to log in before you can comment on or make changes to this bug.