1067453 - Typo in testRestrictedProfiles.js

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[(inactive) Jim Chen [:jchen] [:darchons]]

The class name is missing an 's'; should be "org.mozilla.gecko.RestrictedProfiles"

[1] http://mxr.mozilla.org/mozilla-central/source/mobile/android/base/tests/testRestrictedProfiles.js#41

Comment 1

[(inactive) Jim Chen [:jchen] [:darchons]]

Attachment: Fix typo in testRestrictedProfiles.js

r=trivial

Comment 2

[Nick Alexander :nalexander [he/him]]

Is this sufficient?  RestrictedProfiles and getUserRestrictions are not annotated as RobocopTargets.  Does WrapElementForJNI imply that these names won't change and are not Proguarded out?

Comment 3

[(inactive) Jim Chen [:jchen] [:darchons]]

(In reply to Nick Alexander :nalexander from comment #2)
> Is this sufficient?  RestrictedProfiles and getUserRestrictions are not
> annotated as RobocopTargets.  Does WrapElementForJNI imply that these names
> won't change and are not Proguarded out?

Yes, I believe RobocopTarget and WrapElementForJNI are treated very similarly in the Proguard config. I was able to run the test locally after the fix.

Comment 4

[Nick Alexander :nalexander [he/him]]

(In reply to Jim Chen [:jchen :nchen] from comment #3)
> (In reply to Nick Alexander :nalexander from comment #2)
> > Is this sufficient?  RestrictedProfiles and getUserRestrictions are not
> > annotated as RobocopTargets.  Does WrapElementForJNI imply that these names
> > won't change and are not Proguarded out?
> 
> Yes, I believe RobocopTarget and WrapElementForJNI are treated very
> similarly in the Proguard config. I was able to run the test locally after
> the fix.

I agree with your analysis, but no amount of local verification is sufficient to prove that your annotations are sufficient.  Proguard symbol stripping is non-deterministic; running locally says nothing about running on buildbots, and certainly guarantees nothing in the future.

Comment 5

[(inactive) Jim Chen [:jchen] [:darchons]]

I agree with Proguard being hard to predict. Running the test locally was just another data point to support the patch's correctness. On the other hand, if our Proguard annotations are behaving wildly differently between buildbot and local builds, we should look into it because that would make development very difficult IMO.

Comment 6

[Nick Alexander :nalexander [he/him]]

(In reply to Jim Chen [:jchen :nchen] from comment #5)
> I agree with Proguard being hard to predict. Running the test locally was
> just another data point to support the patch's correctness. On the other
> hand, if our Proguard annotations are behaving wildly differently between
> buildbot and local builds, we should look into it because that would make
> development very difficult IMO.

Known issue, and I intend to do *something* because this will get much worse when we start landing deeper instrumentation tests.  But I'm not hearing words that you understand what I'm saying.  Proguard is unpredictable.  There is no way to make local and buildbot builds *not* behave "wildly differently".  Your mental model should be that it is like PGO: there's no expectation that two consecutive runs in exactly the same environment produce the same output.

Comment 7

[Mark Finkle (:mfinkle) (use needinfo?)]

(In reply to Nick Alexander :nalexander from comment #6)

> Known issue, and I intend to do *something* because this will get much worse
> when we start landing deeper instrumentation tests.  But I'm not hearing
> words that you understand what I'm saying.  Proguard is unpredictable. 
> There is no way to make local and buildbot builds *not* behave "wildly
> differently".  Your mental model should be that it is like PGO: there's no
> expectation that two consecutive runs in exactly the same environment
> produce the same output.

Sounds a bit like quantum mechanics. Let's keep plowing ahead like nothing's wrong until one of us gets stuck in the transporter's pattern buffer.

Comment 8

[(inactive) Jim Chen [:jchen] [:darchons]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/33bef5d1f9bb

Comment 9

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/33bef5d1f9bb