XPC shell test failures due to bug 755724

RESOLVED FIXED

Status

()

defect
P3
normal
RESOLVED FIXED
7 years ago
7 years ago

People

(Reporter: jimm, Assigned: jimm)

Tracking

Trunk
x86_64
All
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [metro-it1])

Attachments

(1 attachment, 2 obsolete attachments)

When we attempt to run XPCShell tests, there's a step where firefox resources are upacked and moved around:

========= Finished unpack tests (results: 0, elapsed: 23 secs) (at 2012-09-06 14:12:01.686386) =========

========= Started 'bash -c ...' warnings (results: 1, elapsed: 0 secs) (at 2012-09-06 14:12:01.690289) =========
'bash' '-c' u'if [ ! -d firefox/plugins ]; then mkdir firefox/plugins; fi && cp bin/xpcshell.exe firefox && cp bin/ssltunnel.exe firefox && cp -R bin/components/* firefox/components/ && cp -R bin/plugins/* firefox/plugins/ && python -u xpcshell/runxpcshelltests.py --symbols-path=http://ftp.mozilla.org/pub/mozilla.org/firefox/tinderbox-builds/elm-win32/1346957854/firefox-18.0a1.en-US.win32.crashreporter-symbols.zip --manifest=xpcshell/tests/all-test-dirs.list firefox/xpcshell.exe'

Prior to landing bug 755724 firefox resources are stored in the root 'firefox' directory of the zip install. After landing some resources will sit in a sub dir 'browser'.

For example:

before:
  inflating: firefox/components/browsercomps.dll  
after:
  inflating: firefox/browser/components/browsercomps.dll  

Doesn't look like there's a way to tweak this through an mc checkin, buildbot config will need an update.

The experimental code that causes this is currently on elm.
Should add, the test setup failure manifests as such:

cp: target `firefox/components/' is not a directory: No such file or directory
You need this to be fixed ASAP, right?
Assignee: nobody → armenzg
(In reply to Armen Zambrano G. [:armenzg] from comment #2)
> You need this to be fixed ASAP, right?

Hey Armen,

If you can make this change on elm for now that would be great. We should keep this open though so that the same change can be made on mc when all this work lands there.
I don't expect to be a clean change though.

BTW could you please point me to a log where it fails? That way I compare if I need to. If it's not landed yet, that's ok.
(In reply to Armen Zambrano G. [:armenzg] from comment #4)
> I don't expect to be a clean change though.
> 
> BTW could you please point me to a log where it fails? That way I compare if
> I need to. If it's not landed yet, that's ok.

https://tbpl.mozilla.org/php/getParsedLog.php?id=15030926&tree=Elm&full=1

Glandium's patch set is on elm so any of the xpcshell 'X' logs will show this.
Priority: -- → P2
This has a slightly lower priority with some work that has come my way.
I should be able to grab it by the end of next week.
Please raise concerns if this derails your schedule and can try to adjust.
Priority: P2 → P3
The first obvious reason why xpcshell tests fail is because they don't find application data, and we need to pass --app-path to runxpcshelltests.py, but that raises two questions:
  - are we going to have xpcshell tests for webapprt and/or metro? if we do, then we need a way to run some tests with some value of --app-path and other tests with another.
  - should we fix toolkit xpcshell tests that rely on preferences set in browser/? (I know, for instance, that some places tests rely on the places prefs that are only set in browser/ (which, btw, means these tests fail when run with a build of xulrunner, for instance))

The trouble with xpcshell tests is that there are two different ways to run them, the in-tree one, with rules in testing/testsuite-targets.mk, used by developers, and an out-of-tree one, in tinderbox scripts, used by tinderbox builds.

This means we need extra care not to break running xpcshell tests on unrelated branches when fixing the issue for m-c, and it also means it makes testing on try more difficult.
I unfortunately don't know the answers to those questions: it seems to me that we don't want toolkit tests to depend on browser prefs: I don't know if the correct answer is to move the prefs, set test-specific default prefs, or move the tests to browser. If the correct answer is "move the tests to browser" then we will probably end up with similar webapprt/metro tests. But in general I don't think that we should really avoid app-specific xpcshell tests.
Posted patch buildbotcustom patch v.1 (obsolete) — Splinter Review
Before we can get to looking at test problems, we have to get past buildbot's problems with the new file layout. This turned to be a pretty trivial issue. When xpcshell tests run, we go through steps to set of the environment:

1) download firefox and tests zips
2) upack zips

unzip -oq firefox-19.0a1.en-US.win32.zip
unzip -oq firefox-19.0a1.en-US.win32.tests.zip bin* certs* modules* xpcshell*

3) setup some directories and run the tests

In this step, things break because buildbot expects that the firefox unpack will create a firefox/components directory. This dir is in the firefox zip, but it's now specific to the app dirs so it's down in firefox/browser/. and firefox/metro/..

So we need to create a platform components dir buildbot can unzip into. We ran into a similar problem with the plugins dir at some point -

http://mxr.mozilla.org/build/source/buildbotcustom/steps/unittest.py#450

if [ ! -d firefox/plugins ]; then mkdir firefox/plugins; fi &&
cp bin/xpcshell.exe firefox &&
cp bin/ssltunnel.exe firefox &&
cp -R bin/components/* firefox/components/ &&
cp -R bin/plugins/* firefox/plugins/ &&
python -u xpcshell/runxpcshelltests.py --symbols-path=http://ftp.moz...porter-symbols.zip --manifest=xpcshell/tests/all-test-dirs.list firefox/xpcshell.exe

The fix here is to do the same for components - 

if [ ! -d firefox/plugins ]; then mkdir firefox/plugins; fi &&
if [ ! -d firefox/components ]; then mkdir firefox/components; fi &&
cp bin/xpcshell.exe firefox &&
..

Attached is a patch to (what I think!) is the right python script in the buildbotcustom repo. This fix should be safe for all our builders.

--

Note this fix gets us past the buildbot problem so we can see what test failures we have to deal with. For example in my local run I had some problems with the updater tests. We need to get past this buildbot issue though before we can sort these out.
Attachment #679265 - Flags: review?(armenzg)
Attachment #679265 - Attachment is patch: true
Posted patch buildbotcustom patch v.1 (obsolete) — Splinter Review
oops, sorry, didn't save my local changes, this has the proper line ending.
Attachment #679265 - Attachment is obsolete: true
Attachment #679265 - Flags: review?(armenzg)
Attachment #679267 - Flags: review?(armenzg)
Attachment #679267 - Attachment is patch: true
Here's a recent elm log - 

https://tbpl.mozilla.org/php/getParsedLog.php?id=16827385&tree=Elm&full=1

the error is

cp: target `firefox/components/' is not a directory: No such file or directory
This whole step is horrible. I keep hoping we can get rid of it, but xpcshell is so finicky.
(In reply to Ted Mielczarek [:ted.mielczarek] from comment #12)
> This whole step is horrible. I keep hoping we can get rid of it, but
> xpcshell is so finicky.

We should file a follow up on general xpcshell changes. This bug needs to stay narrowly targeted at getting tests running on elm.
For future work, how can we get closer what developers run VS what buildbot runs?

Should we replace the shell chain [1] for a python script that would allow buildbot to be agnostic of what happens internally?

WRT to this current bug it seems that the patch is correct and should be harmless but I would like to test it first.

[1] http://mxr.mozilla.org/build/source/buildbotcustom/steps/unittest.py#450
(In reply to Armen Zambrano G. [:armenzg] from comment #14)
> WRT to this current bug it seems that the patch is correct and should be
> harmless but I would like to test it first.

How much testing do we have to do to get something like this landed in production? Is it a few staging runs or is there more to it?
I just wanted to see it work once.

It went red unfortunately.It seems an easy fix.:
'bash' '-c' 'if [ ! -d firefox/plugins ]; then mkdir firefox/plugins; fi && && if [ ! -d firefox/components ]; then mkdir firefox/components; fi && cp bin/xpcshell.exe firefox && cp bin/ssltunnel.exe firefox && cp -R bin/components/* firefox/components/ && cp -R bin/plugins/* firefox/plugins/ && python -u xpcshell/runxpcshelltests.py --symbols-path=http://ftp.mozilla.org/pub/mozilla.org/firefox/tinderbox-builds/mozilla-central-win32/1351034355/firefox-19.0a1.en-US.win32.crashreporter-symbols.zip --manifest=xpcshell/tests/all-test-dirs.list firefox/xpcshell.exe'
...
bash: -c: line 0: syntax error near unexpected token `&&'
bash: -c: line 0: `if [ ! -d firefox/plugins ]; then mkdir firefox/plugins; fi && && if [ ! -d firefox/components ]; then mkdir firefox/components; fi && cp bin/xpcshell.exe firefox && cp bin/ssltunnel.exe firefox && cp -R bin/components/* firefox/components/ && cp -R bin/plugins/* firefox/plugins/ && python -u xpcshell/runxpcshelltests.py --symbols-path=http://ftp.mozilla.org/pub/mozilla.org/firefox/tinderbox-builds/mozilla-central-win32/1351034355/firefox-19.0a1.en-US.win32.crashreporter-symbols.zip --manifest=xpcshell/tests/all-test-dirs.list firefox/xpcshell.exe'
r=me

It is modified from your patch by removing " && " since we already have a " && ".join() at the beginning :)

I've landed it as 5b2d191120bb on default.

I will be doing a reconfiguration of the masters this morning.
Attachment #679267 - Attachment is obsolete: true
Attachment #679267 - Flags: review?(armenzg)
Attachment #679657 - Flags: review+
Comment on attachment 679267 [details] [diff] [review]
buildbotcustom patch v.1

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

::: steps/unittest.py
@@ +447,5 @@
>          bin_extension = ""
>          if platform.startswith('win'):
>              bin_extension = ".exe"
>          script = " && ".join(["if [ ! -d %(exedir)s/plugins ]; then mkdir %(exedir)s/plugins; fi",
> +                  "&& if [ ! -d %(exedir)s/components ]; then mkdir %(exedir)s/components; fi",

I see, so the " && ".join handles prepending '&&', so 

"&& if [ ! -d %(exedir)s/components ]; then mkdir %(exedir)s/components; fi",

should be

"if [ ! -d %(exedir)s/components ]; then mkdir %(exedir)s/components; fi",
ah, heh, we both got it. Thanks!
The masters are being reconfigured and this will soon be live :)
Whiteboard: leave-open
I've disabled one test that was hanging to get a more complete picture. Doesn't look very pretty at this point, for example in extensions I see

INFO | Result summary:
INFO | Passed: 136
INFO | Failed: 133
INFO | Todo: 0

:/
I think what we need here is some sort of an environment variable that identifies when we are building / running tests with the new packager. This way we can touch up tests so that they work in both cases.

For the browser prefs issues, looks like a common step in toolkit test code is to create the prefs you need. I've only looked at a few failure of this type so far though.
jimm, I will let you keep this bug around for the discussion part of the bug.

AFAIK, the releng fix got deployed and it is running everywhere.

Once there is an action for me please let me know and re-assign it to me.
Assignee: armenzg → jmathies
I disabled one test which was hanging to got a full run. All in all not too bad, but will take some time to dig through.

https://tbpl.mozilla.org/php/getParsedLog.php?id=16875103&tree=Elm&full=1
Summary: XP shell test setup config will need updating due to bug 755724 → XP shell test failures due to bug 755724
*get
Depends on: 810299
Depends on: 810307
Depends on: 810309
Depends on: 810617
Depends on: 810704
Depends on: 810609
Note, bug 810609 isn't an xpc shell failure but I added it here so I can keep track of it. It was introduced in the most recent mc->elm merge.
Depends on: 810810
Whiteboard: leave-open → leave-open, metro-it1
Whiteboard: leave-open, metro-it1 → [leave-open][metro-it1]
Depends on: 815256
Component: Release Engineering: Automation (General) → General
OS: Windows 7 → All
Product: mozilla.org → Core
QA Contact: catlee
Version: other → Trunk
Depends on: 815320
Summary: XP shell test failures due to bug 755724 → XPC shell test failures due to bug 755724
Depends on: 816111
No longer depends on: 815320
Depends on: 817076
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Whiteboard: [leave-open][metro-it1] → [metro-it1]
You need to log in before you can comment on or make changes to this bug.