Last Comment Bug 647482 - jstests looks for config/autoconf.mk in the wrong place
: jstests looks for config/autoconf.mk in the wrong place
Status: RESOLVED FIXED
[fixed-in-tracemonkey]
:
Product: Core
Classification: Components
Component: JavaScript Engine (show other bugs)
: Trunk
: x86 Linux
: -- normal (vote)
: ---
Assigned To: Paul Biggar
:
: Jason Orendorff [:jorendorff]
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2011-04-02 15:13 PDT by Paul Biggar
Modified: 2011-04-26 15:21 PDT (History)
3 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Check cwd too (2.08 KB, patch)
2011-04-02 15:32 PDT, Paul Biggar
n.nethercote: review+
Details | Diff | Splinter Review
fix (1.77 KB, patch)
2011-04-04 00:22 PDT, Paul Biggar
dmandelin: review+
Details | Diff | Splinter Review

Description Paul Biggar 2011-04-02 15:13:29 PDT
In tests/manifest.py, jstests uses config/autoconf.mk. The correct place to find it is in the objdirdir. However, it looks for it in a parent directory of the JS executable, and they are not always the same thing. Specifically, they might not be the same thing if you're testing an installed js. This happens in the SM tests on tinderbox.
Comment 1 Paul Biggar 2011-04-02 15:32:00 PDT
Created attachment 523822 [details] [diff] [review]
Check cwd too

Since we don't actually know where the objdir is (all we really know is the location of the JS executable), this searches the current working directory too.

I've tested this on a normal shell build, and also using the commands and directory structure that tinderbox uses for the SM builds.
Comment 2 Nicholas Nethercote [:njn] 2011-04-03 18:49:34 PDT
Comment on attachment 523822 [details] [diff] [review]
Check cwd too

Stealing:  rs=me.

Next time, if you accidentally cause TB breakage and it doesn't require a tiny fix, it's probably better to back out the change.  It's not good for a multi-orange fix to be waiting on somebody's review.
Comment 4 Paul Biggar 2011-04-03 20:37:43 PDT
(In reply to comment #2) 
> Next time, if you accidentally cause TB breakage and it doesn't require a tiny
> fix, it's probably better to back out the change.  It's not good for a
> multi-orange fix to be waiting on somebody's review.

I'm going to reassert that SM builds are not normal builds, and shouldn't follow normal rules. I feel you should be free to push even when they're orange, and also to leave them orange over the weekend waiting for a review.

Those builds were added to make our lives easier, not harder. I disagree with backing out changes that only affect them - they are NPOTB.
Comment 5 Paul Biggar 2011-04-03 23:56:41 PDT
Not quite fixed. I've disabled jstests as a result.
Comment 6 Paul Biggar 2011-04-03 23:59:05 PDT
Here's the disabling:

http://hg.mozilla.org/tracemonkey/rev/e3168cdb35c8
Comment 7 Paul Biggar 2011-04-04 00:22:02 PDT
Created attachment 523953 [details] [diff] [review]
fix

I had accidentally skipped the cwd, and gone straight to its parent dir. I suspect I omitted testing this on the SM builds after refactoring - this time it's tested on both the SM directory structure and a standard objdir.

(Interestingly, the SM directory structure - in which objdir is parallel to tracemonkey/ - is also an artifact of our strange NSPR usage.)
Comment 8 Paul Biggar 2011-04-17 10:07:25 PDT
http://hg.mozilla.org/tracemonkey/rev/47eb8bb3c9a1

There's a final followup patch to actually turn jstests on on tinderbox. But I want to run tests again before I push that. Hence not [fixed-in-tracemonkey].
Comment 9 Paul Biggar 2011-04-22 07:44:34 PDT
|make jstests| works locally (except for Date tests, since I am now in a cursed timezone).

http://hg.mozilla.org/tracemonkey/rev/6f1a25c4b459
Comment 10 Paul Biggar 2011-04-22 10:55:34 PDT
This failed on some secondary configurations (SM builds). A fix is coming after bug 652127 is fixed.
Comment 11 Paul Biggar 2011-04-23 12:54:16 PDT
Disabled on some afflicted platforms:

   http://hg.mozilla.org/tracemonkey/rev/5f3e21f70465

Disabled jstests in all configurations:

   http://hg.mozilla.org/tracemonkey/rev/a03a4fea1679
Comment 12 Paul Biggar 2011-04-24 03:06:33 PDT
This particular problem is fixed (jstests looking for config/autoconf.mk in the wrong place), but jstests is not re-enabled yet. So marking this fixed, and moving the rest to a follow-on (bug 652408).

Note You need to log in before you can comment on or make changes to this bug.