930188 - fix talos to work with newer method of robocop naming

Status: RESOLVED FIXED

(Testing :: Talos, defect)

Description

[Joel Maher ( :jmaher ) (UTC -8)]

as per: https://hg.mozilla.org/integration/fx-team/diff/cf634a1ac73e/testing/mochitest/runtestsremote.py

Comment 1

[Joel Maher ( :jmaher ) (UTC -8)]

Attachment: org.mozilla.gecko for robocop (1.0)

Comment 2

[Nick Alexander :nalexander [he/him]]

Comment on attachment 821653 [details] [diff] [review]
org.mozilla.gecko for robocop (1.0)

I see no reason this would be a problem, but why isn't this "am instrument" logic abstracted out?

Also, let's spin a try build with bnicholson's patch and this patch to make sure we really have gotten all the occurrences.

Comment 3

[Nick Alexander :nalexander [he/him]]

(In reply to Nick Alexander :nalexander from comment #2)
> Comment on attachment 821653 [details] [diff] [review]
> org.mozilla.gecko for robocop (1.0)
> 
> I see no reason this would be a problem, but why isn't this "am instrument"
> logic abstracted out?

Sorry, unclear: I see no reason this patch would fail.  I would like it so we change one instance for the "am instrument" command line, not N > 1.

Comment 4

[Joel Maher ( :jmaher ) (UTC -8)]

https://hg.mozilla.org/build/talos/rev/f2dcb113bb0b

Comment 5

[Joel Maher ( :jmaher ) (UTC -8)]

to test on try server, you can edit testing/talos/talos.json and set revision (http://dxr.mozilla.org/mozilla-central/source/testing/talos/talos.json#l8) to be "default".  doing this will pick up the latest changes in talos.

Comment 6

[Nick Alexander :nalexander [he/him]]

http://tbpl.mozilla.org/?tree=Try&rev=d1c72e65f582

Comment 7

[Nick Alexander :nalexander [he/him]]

(In reply to Nick Alexander :nalexander from comment #6)
> http://tbpl.mozilla.org/?tree=Try&rev=d1c72e65f582

So, this try build isn't very happy.  bnicholson, your patch would change org.mozilla.fennec.tests.test* into org.mozilla.gecko.tests.test*, right?  If so, we'll need another talos update 'cuz I missed that.

Comment 8

[Brian Nicholson (:bnicholson)]

(In reply to Nick Alexander :nalexander from comment #7)
> (In reply to Nick Alexander :nalexander from comment #6)
> > http://tbpl.mozilla.org/?tree=Try&rev=d1c72e65f582
> 
> So, this try build isn't very happy.  bnicholson, your patch would change
> org.mozilla.fennec.tests.test* into org.mozilla.gecko.tests.test*, right? 
> If so, we'll need another talos update 'cuz I missed that.

Yeah, that's what the patch in bug 905703 does. Joel's patch looks correct to me -- are there other changes that we missed?

Comment 9

[Nick Alexander :nalexander [he/him]]

(In reply to Brian Nicholson (:bnicholson) from comment #8)
> (In reply to Nick Alexander :nalexander from comment #7)
> > (In reply to Nick Alexander :nalexander from comment #6)
> > > http://tbpl.mozilla.org/?tree=Try&rev=d1c72e65f582
> > 
> > So, this try build isn't very happy.  bnicholson, your patch would change
> > org.mozilla.fennec.tests.test* into org.mozilla.gecko.tests.test*, right? 
> > If so, we'll need another talos update 'cuz I missed that.
> 
> Yeah, that's what the patch in bug 905703 does. Joel's patch looks correct
> to me -- are there other changes that we missed?

From the logs:

11:28:28     INFO -  FIRE PROC: ' "MOZ_CRASHREPORTER_SHUTDOWN=1,MOZ_CRASHREPORTER_NO_REPORT=1,NO_EM_RESTART=1" am instrument -w -e deviceroot /mnt/sdcard/tests -e class org.mozilla.fennec.tests.testCheck2 org.mozilla.roboexample.test/org.mozilla.fennec.FennecInstrumentationTestRunner'

So I did something wrong bumping the talos rev.  I didn't use 'default', I used 'f2dcb113bb0b' (from jmaher's https://bugzilla.mozilla.org/show_bug.cgi?id=930188#c4).  jmaher, should that have worked?  Should I repush with 'default'?

Comment 10

[Joel Maher ( :jmaher ) (UTC -8)]

my testing on try was incorrect, I was pointing at the wrong talos.zip.  Running this locally, I get this error:

command:
am instrument -w -e deviceroot /mnt/sdcard/tests -e class org.mozilla.gecko.tests.testPan org.mozilla.roboexample.test/org.mozilla.gecko.FennecInstrumentationTestRunner'

result:
10-25 14:46:23.228 W/ActivityManager( 1423): Unable to find instrumentation info for: ComponentInfo{org.mozilla.roboexample.test/org.mozilla.gecko.FennecInstrumentationTestRunner}


command:
am instrument -w -e deviceroot /mnt/sdcard/tests -e class org.mozilla.gecko.tests.testPan org.mozilla.roboexample.test/org.mozilla.fennec_jmaher.FennecInstrumentationTestRunner


result:
10-25 14:48:04.798 W/ActivityManager( 1423): Error in app org.mozilla.fennec_jmaher running instrumentation ComponentInfo{org.mozilla.roboexample.test/org.mozilla.fennec_jmaher.FennecInstrumentationTestRunner}:
10-25 14:48:04.798 W/ActivityManager( 1423):   java.lang.RuntimeException
10-25 14:48:04.798 W/ActivityManager( 1423):   java.lang.RuntimeException: Could not find test class. Class: org.mozilla.gecko.tests.testPan


so I either need a new patch to make this work, or something else is broken.  This is with mozilla-central local build from yesterday.

I am going to back out the patch I landed until we get this figured out.

Comment 11

[Brian Nicholson (:bnicholson)]

Do you have the patches in bug 905703 applied with it? Bug 905703 changes the package name for these classes. It hasn't landed in m-c yet since there are talos failures, which hopefully this bug will fix.

Comment 12

[Joel Maher ( :jmaher ) (UTC -8)]

I did not have those patches, I can look into this next week when I have more time.

Comment 13

[Joel Maher ( :jmaher ) (UTC -8)]

oh, I ran with these patches and the original patch, it worked as expected.  The problem with the try run you guys did was the talos.*.zip file that was being used.  Here is my try server run:
https://tbpl.mozilla.org/?tree=Try&rev=1d282c24aaaa

Comment 14

[Nick Alexander :nalexander [he/him]]

(In reply to Joel Maher (:jmaher) from comment #13)
> oh, I ran with these patches and the original patch, it worked as expected. 
> The problem with the try run you guys did was the talos.*.zip file that was
> being used.  Here is my try server run:
> https://tbpl.mozilla.org/?tree=Try&rev=1d282c24aaaa

OK, great.  It looks like Bug 905703 doesn't need any more work.  We'd like to land it ASAP -- it's blocking other work -- but it needs to land with these talos changes enabled.  jmaher, does that just mean pushing with to mozilla-inbound with the updated talos revision in testing/talos/talos.json?  Or do we need to do something with the talos.zip file as well?  Attaching a patch to this bug would be best.

Comment 15

[Joel Maher ( :jmaher ) (UTC -8)]

I need to land the talos patch on the talos repo, then I can attach a m-c patch to this bug which will work.  I need to do a talos update today and I can do this right afterwards.  Since this requires other code to land, we shouldn't mix it up in a talos update in case it gets backed up.

Comment 16

[Joel Maher ( :jmaher ) (UTC -8)]

landed on talos again:
https://hg.mozilla.org/build/talos/rev/0987e4cbd219

Comment 17

[Joel Maher ( :jmaher ) (UTC -8)]

once bug 934517 is landed, I will verify it all is working as expected on try server and send a patch to land with the patches in bug 905703.

Comment 18

[Joel Maher ( :jmaher ) (UTC -8)]

Attachment: updated talos.json references to pick up newer robocop naming (1.0)

This is the patch needed to land with the patches in bug 905703.

Comment 19

[Nick Alexander :nalexander [he/him]]

Comment on attachment 828577 [details] [diff] [review]
updated talos.json references to pick up newer robocop naming (1.0)

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

https://tbpl.mozilla.org/?tree=Try&rev=64d2089ac862

Comment 20

[Nick Alexander :nalexander [he/him]]

This patch was folded into https://hg.mozilla.org/integration/mozilla-inbound/rev/082c445306e8 so that we don't break bisect. jmaher, thanks for all your work on this!