Closed Bug 1056045 Opened 10 years ago Closed 9 years ago

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

Categories

(Testing Graveyard :: Mozmill, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: andrei, Assigned: andrei)

References

Details

(Whiteboard: [mozmill-2.1+])

Attachments

(1 file, 3 obsolete files)

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.
No longer blocks: 1043391
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.
Assignee: nobody → andrei.eftimie
Status: NEW → ASSIGNED
Summary: Bump mozmill dependency versions → Update deps and tests for the new manifestparser changes in regards to [parent:]
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.
Attachment #8480619 - Flags: review?(hskupin)
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.
Attachment #8480619 - Flags: review?(hskupin) → review+
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.
Attachment #8480619 - Flags: review+ → review-
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
Blocks: 1061982
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.
Attachment #8480619 - Attachment is obsolete: true
(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.
Flags: needinfo?(jmaher)
Flags: needinfo?(jgriffin)
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.
Flags: needinfo?(jmaher)
Flags: needinfo?(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.
Depends on: 1086678
Jonathan thanks a lot!! If you can get a new release out that would be awesome too.
(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.
It's better that we fix manifestparser to be smart about such cases.  I'll release a new version today.
Depends on: 1087711
Andrei, I have released manifestparser 0.8 right now. Can you please update this patch and check if it works now? Thanks.
Flags: needinfo?(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.
Updated patch.

I can't test this ATM due to OSX 10.10 vs mozrunner 5.35
I'll test it tomorrow.
Attachment #8503078 - Attachment is obsolete: true
Attachment #8510351 - Flags: feedback?(hskupin)
(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.
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.
Depends on: 1088029
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?
Attachment #8510351 - Flags: feedback?(hskupin) → feedback+
(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
Flags: needinfo?(andrei.eftimie)
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.
Attachment #8510351 - Attachment is obsolete: true
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.
Depends on: 1111682
Whiteboard: [mozmill-2.1+]
Attached patch Final patchSplinter Review
The final patch with only small changes to setup.py.
https://github.com/mozilla/mozmill/commit/d79cbdc805bccb99c184852c25001f3987ecc1f0 (master)

Thanks Andrei for putting this patch together! Finally we got it!
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
No longer blocks: 1110837
Product: Testing → Testing Graveyard
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: