911302 - if talos has --fennecIDs specified on the commandline, ensure that it is a robocop based test

Status: RESOLVED FIXED

(Testing :: Talos, defect)

Description

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

if we use --fennecIDs on the command line, we set the profile up differently.  This isn't good and we do unnecessary actions.

we should check if fennecIDs is enabled that one of these tests are being used:
trobopan, tcheckerboard, tprovider, tcheck2

Ideally we would have an attribute on the tests in test.py so we could query the test attribute and the commandline flags!

Comment 1

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

This bug needs to have talos/test.py modified to have a true/false attribute on all tests, and if we are using robocop (a line similar to this):
http://hg.mozilla.org/build/talos/file/tip/talos/test.py#l138

you will add another attribute to the class:
fennecIDs = True

We can default it to False.

-------------

Once that is done, we need to modify the PerfConfigurator.py so that when we have --fennecIDs defined on the command line, but it is False on the test, we throw a configuration error.  This will take place in the validate function:
http://hg.mozilla.org/build/talos/file/tip/talos/PerfConfigurator.py#l358

you can test this by changing the commandline parameters to add --fennecIDs <path to file>, and testing various tests which do and do not support this argument.

Comment 2

[Vaibhav (:vaibhav1994)]

I would like to work on this bug. I have talos running locally. Kindly let me know how I have to proceed on this bug.

Comment 3

[Vaibhav (:vaibhav1994)]

Attachment: This patch ensures that when --fennecIDs is specified on the commandline, it is a robocop based test.

Comment 4

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

Comment on attachment 8365140 [details] [diff] [review]
This patch ensures that when --fennecIDs is specified on the commandline, it is a robocop based test.

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

close, a lot of little details to sort out- clean it up and I would love to review it again!

::: talos/PerfConfigurator.py
@@ +368,5 @@
>          self.config['remote'] = self.remote
>  
>          if self.remote:
>              # setup remote
> +                

please remove this extra whitespace added here

@@ +520,4 @@
>          for test in self.config.get('tests', []):
>              # robopan is the only robocop based extension which uses roboextender
>              if self.config.get('fennecIDs', '') and test['name'] == 'trobopan':
>                  self.config['extensions'] = ['${talos}/mobile_extensions/[e-mail]']

you don't need to loop through the tests twice, just put this if condition inside the next for loop.

::: talos/ttest.py
@@ +216,5 @@
>              print "Remote Device Error: Error copying files for robocop setup"
>              raise
>  
>  
> +

please remove this extra line.

::: tests/test_PerfConfigurator.py
@@ +89,5 @@
>          error_tests = {'--activeTests':{'error':ConfigurationError,
>                                          'args':['--activeTests', 'badtest', '--develop', '-e', ffox_path, "-o", outfile],
>                                          'except_fault' : 'invalid --activeTest raised an error that is not ConfigurationError',
>                                          'non_raises_fault' : 'invalid --activeTest passed test'},
> +                       

please remove this extra line

@@ +91,5 @@
>                                          'except_fault' : 'invalid --activeTest raised an error that is not ConfigurationError',
>                                          'non_raises_fault' : 'invalid --activeTest passed test'},
> +                       
> +                       '--fennecIDs':{'error':ConfigurationError,
> +                                        'args':['--activeTests', 'ts', '--develop', '-e', ffox_path,'--fennecIDs',ffox_path,"-o", outfile],

nit: add whitespaces after ,'s to be consistent.

@@ +106,5 @@
> +                                        'except_fault' : 'invalid --fennecIDs raised an error that is not ConfigurationError',
> +                                        'non_raises_fault' : 'invalid --fennecIDs passed test'},
> +
> +                       
> +                       

nit, remove the extra whitepspace here, a blank line should be a blank line.

@@ +111,5 @@
>                         '--filter':{'error':ConfigurationError,
>                                     'args' : ['--activeTests', 'ts', '--develop',  '-e', ffox_path, '--filter', 'badfilter', '-o', outfile],
>                                     'except_fault' : 'invalid --filter raised an error that is not ConfigurationError',
>                                     'non_raises_fault' : 'invalid --filter passed test'},
> +                       

remove the whitespace here

Comment 5

[Vaibhav (:vaibhav1994)]

Attachment: Cleaned up fennecIDs.patch

This patch ensures that when --fennecIDs is specified on the commandline, it is a robocop based test.

Comment 6

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

Comment on attachment 8365353 [details] [diff] [review]
Cleaned up fennecIDs.patch

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

this looks great.  Do you want to commit this to the repository?

Comment 7

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

thanks, I have landed this:
https://hg.mozilla.org/build/talos/rev/b2006ffcdf4c