1056045 - Update deps and tests for the new manifestparser changes in regards to [parent:]

Status: RESOLVED FIXED

(Testing Graveyard :: Mozmill, defect)

Description

[Andrei Eftimie]

For mozmill 2.1 we'll need to bump some dependency versions:
- mozrunner (for mozprofile and manifestparser changes)
- wptserve==1.2 (mainly to declutter the console log)

We'll have a new mozrunner version available with bug 1056037.

Comment 1

[Andrei Eftimie]

We won't release a new mozrunner version and mozprofile is handled in a different bug, lets update the manifestparser version dependency and add the relevant changes to our mutt test files.

Comment 2

[Andrei Eftimie]

Attachment: 0001-Bug-1056045-Update-mutt-test-files-and-manifestparse.patch

To properly test this you'll need bug 1023790 and bug 1056037.
All mutt tests are passing.

Note: there are a few disconnected metro tests in there (by disconnected I mean not included in the main manifest.ini tree). I've left them out of this. Ill file a bug later on to see what we need to do about them.

Comment 3

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Comment on attachment 8480619 [details] [diff] [review]
0001-Bug-1056045-Update-mutt-test-files-and-manifestparse.patch

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

Looks fine so far. I will get this tested with your manifestparser patch.

::: mozmill/setup.py
@@ +8,5 @@
>  PACKAGE_NAME = "mozmill"
>  PACKAGE_VERSION = "2.1-dev"
>  
>  deps = ['jsbridge == 3.1-dev',
> +        'manifestparser == 0.61',

FYI: This might need an update, if someone else releases another version before we get the manifestparser change in.

Comment 4

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Comment on attachment 8480619 [details] [diff] [review]
0001-Bug-1056045-Update-mutt-test-files-and-manifestparse.patch

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

This patch needs a little update for the version of manifestparser which is 0.7. Andrei, can you please fix that, and re-test if possible? Thanks.

::: mozmill/setup.py
@@ +8,5 @@
>  PACKAGE_NAME = "mozmill"
>  PACKAGE_VERSION = "2.1-dev"
>  
>  deps = ['jsbridge == 3.1-dev',
> +        'manifestparser == 0.61',

FYI: This might need an update, if someone else releases another version before we get the manifestparser change in.

Comment 5

[Andrei Eftimie]

manifestparser 0.7 seems to contain one change that breaks mozmill.
Bug 1061982 - manifestparser changes to support conditional subsuites, r=jmaher
Introduced subsuites, and we currently fail in mozmill if we try to run a test directly with -t.

This fails:
> mozmill -t %test% -b %binary%
with:
> Traceback (most recent call last):
>   File "/Users/andrei.eftimie/work/mozilla/env/mozmill/bin/mozmill", line 9, in <module>
>     load_entry_point('mozmill==2.1-dev', 'console_scripts', 'mozmill')()
>   File "/Users/andrei.eftimie/work/mozilla/mozmill/src/mozmill/mozmill/__init__.py", line 938, in cli
>     CLI(args).run()
>   File "/Users/andrei.eftimie/work/mozilla/mozmill/src/mozmill/mozmill/__init__.py", line 918, in run
>     tests = self.manifest.active_tests(**mozinfo.info)
>   File "/Users/andrei.eftimie/work/mozilla/env/mozmill/lib/python2.7/site-packages/manifestparser-0.7-py2.7.egg/manifestparser/manifestparser.py", line 1160, in active_tests
>     if ',' in subsuite:
> TypeError: argument of type 'NoneType' is not iterable

Comment 6

[Andrei Eftimie]

Attachment: 0002-Bug-1056045-Update-mutt-test-files-and-manifestparse.patch

In the meantime here's the updated version of the patch.

I guess the subsuite failure might be fixed in mozmill directly. I haven't investigated the issue, and will not have time today. If this is urgent feel free to try fixing it. I will try to have another look next week.

Comment 7

[Henrik Skupin [:whimboo][⌚️UTC+2]]

(In reply to Andrei Eftimie from comment #5)
> manifestparser 0.7 seems to contain one change that breaks mozmill.
> Bug 1061982 - manifestparser changes to support conditional subsuites,
> r=jmaher
> Introduced subsuites, and we currently fail in mozmill if we try to run a
> test directly with -t.
> 
> This fails:
> > mozmill -t %test% -b %binary%
> with:
> > Traceback (most recent call last):
> >   File "/Users/andrei.eftimie/work/mozilla/env/mozmill/bin/mozmill", line 9, in <module>
> >     load_entry_point('mozmill==2.1-dev', 'console_scripts', 'mozmill')()
> >   File "/Users/andrei.eftimie/work/mozilla/mozmill/src/mozmill/mozmill/__init__.py", line 938, in cli
> >     CLI(args).run()
> >   File "/Users/andrei.eftimie/work/mozilla/mozmill/src/mozmill/mozmill/__init__.py", line 918, in run
> >     tests = self.manifest.active_tests(**mozinfo.info)
> >   File "/Users/andrei.eftimie/work/mozilla/env/mozmill/lib/python2.7/site-packages/manifestparser-0.7-py2.7.egg/manifestparser/manifestparser.py", line 1160, in active_tests
> >     if ',' in subsuite:
> > TypeError: argument of type 'NoneType' is not iterable

Before I can dig into that one, Jonathan or Joel, does one of you have an idea what's going on?

Andrei, we would need another update of the patch given that I just landed a patch for moving to manifestparser. Thanks.

Comment 8

[Joel Maher ( :jmaher ) (UTC -8) (PTO back normal Nov 17)]

An interesting problem, it appears that manifestparser.py had a bug in it.  From looking at the source code we could do a better job in a few areas of the code to ensure that we have checked for existing values before accessing them.

Comment 9

[Jonathan Griffin (:jgriffin)]

I'm not quite sure how you end up with a subsuite of None, mozmill may be using manifestparser in some idiosyncratic way.  But it's an easy enough fix to guard against this; I'll put up a patch later.

Comment 10

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Jonathan thanks a lot!! If you can get a new release out that would be awesome too.

Comment 11

[Henrik Skupin [:whimboo][⌚️UTC+2]]

(In reply to Jonathan Griffin (:jgriffin) from comment #9)
> I'm not quite sure how you end up with a subsuite of None, mozmill may be
> using manifestparser in some idiosyncratic way.  But it's an easy enough fix
> to guard against this; I'll put up a patch later.

Mozmill always creates a TestManifest instance even if no manifests have been specified. So in those cases we end-up with: TestManifest(manifests=None, strict=False)

Code: https://github.com/mozilla/mozmill/blob/master/mozmill/mozmill/__init__.py#L730

So in case of -t, we extend the tests via manifest.tests.extend(test). If that is an inappropriate usage of the class please let us know. But I think it's nothing we want to change anymore.

Comment 12

[Jonathan Griffin (:jgriffin)]

It's better that we fix manifestparser to be smart about such cases.  I'll release a new version today.

Comment 13

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Andrei, I have released manifestparser 0.8 right now. Can you please update this patch and check if it works now? Thanks.

Comment 14

[Andrei Eftimie]

Ugh, seems mozmill/master is broken for me (OSX 10.10 possibly?)
And this patch doesn't apply on hotfix-2.0 cleanly.

I'll find a way to test it tomorrow.

Comment 15

[Andrei Eftimie]

Attachment: 0003-Bug-1056045-Update-mutt-test-files-and-manifestparse.patch

Updated patch.

I can't test this ATM due to OSX 10.10 vs mozrunner 5.35
I'll test it tomorrow.

Comment 16

[Henrik Skupin [:whimboo][⌚️UTC+2]]

(In reply to Andrei Eftimie from comment #14)
> Ugh, seems mozmill/master is broken for me (OSX 10.10 possibly?)
> And this patch doesn't apply on hotfix-2.0 cleanly.

FYI this patch will never land on hotfix-2.0, it will be master only.

Comment 17

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Also mozrunner 5.35 will not really work on OS X 10.10 due to bug 1079890. For now you can test with mozrunner 6.4, but keep in mind that you will see bug 1088029. I will land the patch soon for master and hotfix-2.0. Maybe we should wait a bit here.

Comment 18

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Comment on attachment 8510351 [details] [diff] [review]
0003-Bug-1056045-Update-mutt-test-files-and-manifestparse.patch

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

So far this looks fine. Have you had a chance to check what is currently broken in Mozmill?

Comment 19

[Andrei Eftimie]

(In reply to Henrik Skupin (:whimboo) from comment #18)
> Comment on attachment 8510351 [details] [diff] [review]
> 0003-Bug-1056045-Update-mutt-test-files-and-manifestparse.patch
> 
> Review of attachment 8510351 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> So far this looks fine. Have you had a chance to check what is currently
> broken in Mozmill?

Ugh, mozmill master is completely broken for me.

Both on OSX 10.10, and Ubuntu I don't get any tests run:
> > mozmill -m mutt/mutt/tests/js/testAssertions/manifest.ini -b /Applications/FirefoxNightly.app
> ^/(.*)$
> ^/(.*)\.asis$
> ^/(.*)\.py$
> No handlers could be found for logger "wptserve"
> ^/(.*)$
> mozversion INFO | application_buildid: 20141106030201
> mozversion INFO | application_changeset: 2114ef80f6ae
> mozversion INFO | application_display_name: Nightly
> mozversion INFO | application_id: {ec8030f7-c20a-464f-9b0e-13a3a9e97384}
> mozversion INFO | application_name: Firefox
> mozversion INFO | application_remotingname: firefox
> mozversion INFO | application_repository: https://hg.mozilla.org/mozilla-central
> mozversion INFO | application_vendor: Mozilla
> mozversion INFO | application_version: 36.0a1
> mozversion INFO | platform_buildid: 20141106030201
> mozversion INFO | platform_changeset: 2114ef80f6ae
> mozversion INFO | platform_repository: https://hg.mozilla.org/mozilla-central
> mozversion INFO | platform_version: 36.0a1
> RESULTS | Passed: 0
> RESULTS | Failed: 0
> RESULTS | Skipped: 0

Comment 20

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Comment on attachment 8510351 [details] [diff] [review]
0003-Bug-1056045-Update-mutt-test-files-and-manifestparse.patch

This patch needs a little update. I will hopefully have a working version soon given that Mozmill on master is working again.

Comment 21

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Before we can continue here we need bug 1111682 with an updated mozrunner. The current one would also install ManifestDestiny which results in a conflict in using the right manifestparser.

Comment 22

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Attachment: Final patch

The final patch with only small changes to setup.py.

Comment 23

[Henrik Skupin [:whimboo][⌚️UTC+2]]

https://github.com/mozilla/mozmill/commit/d79cbdc805bccb99c184852c25001f3987ecc1f0 (master)

Thanks Andrei for putting this patch together! Finally we got it!