1429687 - Link unit tests for talos/talos/xtalos/etlparser.py to ./mach python-test

Status: RESOLVED FIXED

(Testing :: Talos, enhancement, P3)

Comment 1

[Ionuț Goldan [:igoldan]]

The unit test residing here [1] isn't run automatically, after one executes ./mach python-test testing/talos/talos/unittests.

I didn't include a fix for this when I landed bug 1423353. There are some import configurations for xtalos package, which pytest seems to dislike. I will address them here.

[1] testing\talos\talos\unittests\test_xtalos\test_etlparser.py

Comment 2

[Ionuț Goldan [:igoldan]]

Attachment: Bug 1429687 - Fix existing tests

Review commit: https://reviewboard.mozilla.org/r/249574/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/249574/

Comment 3

[Ionuț Goldan [:igoldan]]

Attachment: Bug 1429687 - Fix import issues

Review commit: https://reviewboard.mozilla.org/r/249576/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/249576/

Comment 4

[Ionuț Goldan [:igoldan]]

I ran all the perf tests here: https://treeherder.mozilla.org/#/jobs?repo=try&revision=60c2900b07e0ef7180cdcd5aca9f3196a41219f6

Comment 5

[Robert Wood [:rwood]]

I'll get to these reviews today thanks

Comment 6

[Robert Wood [:rwood]]

Just FYI you may need to rebase, I had a merge conflict applying your first patch (testing/talos/talos/unittests/test_test.py) against the latest inbound.

Comment 7

[Robert Wood [:rwood]]

Comment on attachment 8983741 [details]
Bug 1429687 - Fix existing tests

https://reviewboard.mozilla.org/r/249574/#review256292

Applied locally and ran the modified unit tests, they passed fine (OSX).

R+ with the one issue addressed first please (re: 'preferences'), thanks!

::: testing/talos/talos/unittests/test_config.py:120
(Diff revision 1)
>          assert test_dict['w7_counters'] == counters
>          assert set(test_dict['xperf_counters']) == set(counters + ['counter_extra'])
>  
>  
>  class Test_get_browser_config(object):
> -    required = ('preferences', 'extensions', 'browser_path', 'browser_wait',
> +    required = ('extensions', 'browser_path', 'browser_wait',

Why was 'preferences' removed completely? It is still used inside Talos. Shouldn't it be in the 'optional' list at least?

Comment 8

[Robert Wood [:rwood]]

Comment on attachment 8983742 [details]
Bug 1429687 - Fix import issues

https://reviewboard.mozilla.org/r/249576/#review256294

LGTM. Appiled locally and verified via ./mach python-test testing/talos/talos/unittests/.

Just the one nit. R+.

::: testing/talos/talos/unittests/python.ini:7
(Diff revision 1)
>  subsuite = talos
>  
>  [test_config.py]
>  [test_ffsetup.py]
>  [test_test.py]
> +[test_xtalos/test_etlparser.py]

Nit: should have a blank line at the end of the file

Comment 9

[Ionuț Goldan [:igoldan]]

Comment on attachment 8983741 [details]
Bug 1429687 - Fix existing tests

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/249574/diff/1-2/

Comment 10

[Ionuț Goldan [:igoldan]]

Comment on attachment 8983742 [details]
Bug 1429687 - Fix import issues

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/249576/diff/1-2/

Comment 11

[Ionuț Goldan [:igoldan]]

Comment on attachment 8983742 [details]
Bug 1429687 - Fix import issues

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/249576/diff/2-3/

Comment 12

[Ionuț Goldan [:igoldan]]

Comment on attachment 8983741 [details]
Bug 1429687 - Fix existing tests

https://reviewboard.mozilla.org/r/249574/#review256292

> Why was 'preferences' removed completely? It is still used inside Talos. Shouldn't it be in the 'optional' list at least?

Indeed, this is now optional. I just added it there.

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/c6bf8562cf6d
Fix existing tests r=rwood
https://hg.mozilla.org/integration/autoland/rev/fcbec6eece3f
Fix import issues r=rwood

Comment 14

[Andrei Ciure[:aciure]]

https://hg.mozilla.org/mozilla-central/rev/c6bf8562cf6d
https://hg.mozilla.org/mozilla-central/rev/fcbec6eece3f