1057328 - --app-override dist is broken with message "Could not find path to gmp-fake plugin!"

Status: RESOLVED FIXED

(Testing :: Mochitest, defect)

Description

[:Gijs (he/him)]

http://hg.mozilla.org/mozilla-central/annotate/0b9dd32d1e16/testing/mochitest/runtests.py#l1206

http://hg.mozilla.org/mozilla-central/rev/d32c7255e4b9

bug 1041347

Comment 1

[:Gijs (he/him)]

Also broken on Linux.

Comment 2

[:Gijs (he/him)]

Attachment: get gmp path for app-override tests,

I don't feel happy about how I'm finding this path, but I don't know if we have a better way that deals with platform idiosyncracies... do we?

Comment 3

[(not currently active) Ted Mielczarek]

Comment on attachment 8477367 [details] [diff] [review]
get gmp path for app-override tests,

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

r- because I think this will break when called in other ways. Should be easy to fix though.

::: testing/mochitest/mach_commands.py
@@ +360,5 @@
> +                options.app = self.get_binary_path(where='staged-package')
> +            elif app_override:
> +                options.app = app_override
> +            # Need to fix the location of gmp_fake which might not be shipped in the binary
> +            gmp_fake = self.get_binary_path(what='gmp-fake', validate_exists=False)

Alternately you could do:
bin_path = self.get_binary_path()
gmp_fake = os.path.join(os.path.dirname(bin_path), 'gmp-fake')

::: testing/mochitest/runtests.py
@@ +1161,5 @@
>  
>      return manifest
>  
>    def getGMPPluginPath(self, options):
> +    if options.gmp_path:

I think this is going to break in the non-app-override case, since nothing is setting this property. You can default it to None in mochitest_options.py.

Comment 4

[:Gijs (he/him)]

(In reply to Ted Mielczarek [:ted.mielczarek] from comment #3)
> Comment on attachment 8477367 [details] [diff] [review]
> get gmp path for app-override tests,
> 
> Review of attachment 8477367 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> r- because I think this will break when called in other ways. Should be easy
> to fix though.
> 
> ::: testing/mochitest/mach_commands.py
> @@ +360,5 @@
> > +                options.app = self.get_binary_path(where='staged-package')
> > +            elif app_override:
> > +                options.app = app_override
> > +            # Need to fix the location of gmp_fake which might not be shipped in the binary
> > +            gmp_fake = self.get_binary_path(what='gmp-fake', validate_exists=False)
> 
> Alternately you could do:
> bin_path = self.get_binary_path()
> gmp_fake = os.path.join(os.path.dirname(bin_path), 'gmp-fake')

This seems nicer.

> 
> ::: testing/mochitest/runtests.py
> @@ +1161,5 @@
> >  
> >      return manifest
> >  
> >    def getGMPPluginPath(self, options):
> > +    if options.gmp_path:
> 
> I think this is going to break in the non-app-override case, since nothing
> is setting this property. You can default it to None in mochitest_options.py.

Running this locally without passing any app-override works, actually... but defaulting it explicitly is probably a good idea anyway. :-)

Comment 5

[:Gijs (he/him)]

Attachment: get gmp path for app-override tests,

Is this what you meant?

Comment 6

[(not currently active) Ted Mielczarek]

Comment on attachment 8477406 [details] [diff] [review]
get gmp path for app-override tests,

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

Feels like overkill to have an actual commandline argument that nobody needs, but it's not a big deal.

Comment 7

[:Gijs (he/him)]

remote:   https://hg.mozilla.org/integration/fx-team/rev/ab38c3bfcb8f

Comment 8

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/ab38c3bfcb8f

Comment 9

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/ee3f447df950