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

RESOLVED FIXED

Status

()

Core
JavaScript Engine
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: Paul Biggar, Assigned: Paul Biggar)

Tracking

Trunk
x86
Linux
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [fixed-in-tracemonkey])

Attachments

(1 attachment, 1 obsolete attachment)

fix
1.77 KB, patch
dmandelin
: review+
Details | Diff | Splinter Review
(Assignee)

Description

6 years ago
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.
(Assignee)

Comment 1

6 years ago
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.
Attachment #523822 - Flags: review?(dmandelin)
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.
Attachment #523822 - Flags: review?(dmandelin) → review+
(Assignee)

Comment 3

6 years ago
http://hg.mozilla.org/tracemonkey/rev/43f70a7294fc
Whiteboard: [fixed-in-tracemonkey]
(Assignee)

Comment 4

6 years ago
(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.
(Assignee)

Comment 5

6 years ago
Not quite fixed. I've disabled jstests as a result.
Whiteboard: [fixed-in-tracemonkey]
(Assignee)

Comment 6

6 years ago
Here's the disabling:

http://hg.mozilla.org/tracemonkey/rev/e3168cdb35c8
(Assignee)

Comment 7

6 years ago
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.)
Attachment #523822 - Attachment is obsolete: true
Attachment #523953 - Flags: review?(dmandelin)
Attachment #523953 - Flags: review?(dmandelin) → review+
(Assignee)

Comment 8

6 years ago
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].
(Assignee)

Comment 9

6 years ago
|make jstests| works locally (except for Date tests, since I am now in a cursed timezone).

http://hg.mozilla.org/tracemonkey/rev/6f1a25c4b459
Whiteboard: [fixed-in-tracemonkey]
(Assignee)

Updated

6 years ago
Depends on: 652154
(Assignee)

Updated

6 years ago
Depends on: 652155
(Assignee)

Comment 10

6 years ago
This failed on some secondary configurations (SM builds). A fix is coming after bug 652127 is fixed.
Whiteboard: [fixed-in-tracemonkey]
(Assignee)

Comment 11

6 years ago
Disabled on some afflicted platforms:

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

Disabled jstests in all configurations:

   http://hg.mozilla.org/tracemonkey/rev/a03a4fea1679
(Assignee)

Updated

6 years ago
No longer depends on: 652155
(Assignee)

Updated

6 years ago
No longer depends on: 652154
(Assignee)

Comment 12

6 years ago
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).
Whiteboard: [fixed-in-tracemonkey]
http://hg.mozilla.org/mozilla-central/rev/5f3e21f70465
http://hg.mozilla.org/mozilla-central/rev/a03a4fea1679
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.