979454 - manifest parser needs to support skip-if in the [default] section and || that with the skip-if from the [test] section

Status: RESOLVED FIXED

(Testing :: General, defect)

Description

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

currently manifestparser supports skip-if in the [default] section, but if the [test] has a skip-if, that skip-if overwrites the value in the global.  This is problematic for us when converting android|b2g.json manifests to ini.

Comment 1

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

Attachment: manifest_defaults.patch

Comment 2

[(not currently active) Ted Mielczarek]

Comment on attachment 8385491 [details] [diff] [review]
manifest_defaults.patch

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

Thanks for writing a test!

::: testing/mozbase/manifestdestiny/manifestparser/manifestparser.py
@@ +390,4 @@
>      # interpret the variables
>      def interpret_variables(global_dict, local_dict):
>          variables = global_dict.copy()
> +        if local_dict.get('skip-if', '') and variables.get('skip-if', ''):

I would say "if 'skip-if' in local_dict and 'skip-if' in variables:"

@@ +395,2 @@
>          variables.update(local_dict)
> +            

I think you have goofy whitespace here.

Comment 3

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

Attachment: preserve default skip-if when test has a skip-if also (1.1)

sorry for the rereview, I have had to do some more logic to remove the comments. With this patch, we are good on try server.

Comment 4

[(not currently active) Ted Mielczarek]

Comment on attachment 8385996 [details] [diff] [review]
preserve default skip-if when test has a skip-if also (1.1)

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

::: testing/mozbase/manifestdestiny/manifestparser/manifestparser.py
@@ +390,5 @@
>      # interpret the variables
>      def interpret_variables(global_dict, local_dict):
>          variables = global_dict.copy()
> +        if 'skip-if' in local_dict and 'skip-if' in variables:
> +            local_dict['skip-if'] = "(%s) || (%s)" % (variables['skip-if'].split('#')[0], local_dict['skip-if'].split('#')[0])

Add a test for this behavior? We should probably make this less awful somehow.

Comment 5

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

landed with test for comments:
https://hg.mozilla.org/integration/mozilla-inbound/rev/7373f3e5d57a

Comment 6

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

https://hg.mozilla.org/mozilla-central/rev/7373f3e5d57a