647482 - jstests looks for config/autoconf.mk in the wrong place

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Paul Biggar]

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]

Attachment: 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 [inactive]]

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 3

[Paul Biggar]

http://hg.mozilla.org/tracemonkey/rev/43f70a7294fc

Comment 4

[Paul Biggar]

(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]

Not quite fixed. I've disabled jstests as a result.

Comment 6

[Paul Biggar]

Here's the disabling:

http://hg.mozilla.org/tracemonkey/rev/e3168cdb35c8

Comment 7

[Paul Biggar]

Attachment: 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]

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]

|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]

This failed on some secondary configurations (SM builds). A fix is coming after bug 652127 is fixed.

Comment 11

[Paul Biggar]

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]

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).

Comment 13

[Chris Leary [:cdleary] (not checking bugmail)]

http://hg.mozilla.org/mozilla-central/rev/5f3e21f70465
http://hg.mozilla.org/mozilla-central/rev/a03a4fea1679