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)
Testing Graveyard
Mozmill
Tracking
(Not tracked)
RESOLVED
FIXED
People
(Reporter: andrei, Assigned: andrei)
References
Details
(Whiteboard: [mozmill-2.1+])
Attachments
(1 file, 3 obsolete files)
10.71 KB,
patch
|
Details | Diff | Splinter Review |
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.
Assignee | ||
Comment 1•10 years ago
|
||
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:]
Assignee | ||
Comment 2•10 years ago
|
||
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 3•10 years ago
|
||
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 4•10 years ago
|
||
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-
Assignee | ||
Comment 5•10 years ago
|
||
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
Assignee | ||
Comment 6•10 years ago
|
||
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
Comment 7•10 years ago
|
||
(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)
Comment 8•10 years ago
|
||
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)
Comment 9•10 years ago
|
||
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•10 years ago
|
||
Jonathan thanks a lot!! If you can get a new release out that would be awesome too.
Comment 11•10 years ago
|
||
(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•10 years ago
|
||
It's better that we fix manifestparser to be smart about such cases. I'll release a new version today.
Comment 13•10 years ago
|
||
Andrei, I have released manifestparser 0.8 right now. Can you please update this patch and check if it works now? Thanks.
Updated•10 years ago
|
Flags: needinfo?(andrei.eftimie)
Assignee | ||
Comment 14•10 years ago
|
||
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.
Assignee | ||
Comment 15•10 years ago
|
||
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)
Comment 16•10 years ago
|
||
(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•10 years ago
|
||
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 18•10 years ago
|
||
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+
Assignee | ||
Comment 19•10 years ago
|
||
(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 20•9 years ago
|
||
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
Comment 21•9 years ago
|
||
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
Updated•9 years ago
|
Whiteboard: [mozmill-2.1+]
Comment 22•9 years ago
|
||
The final patch with only small changes to setup.py.
Comment 23•9 years ago
|
||
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
Updated•8 years ago
|
Product: Testing → Testing Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•