344131 - jsDriver.pl not detecting assertion during test as test failure

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Jeff Walden [:Waldo]]

I'll attach the relevant testcase in a second...

Comment 1

[Jeff Walden [:Waldo]]

Attachment: The asserting testcase

The assertion happens in |f3()|, to save people trying to determine that a little time.

Comment 2

[Jeff Walden [:Waldo]]

Blake: the attached testcase is asserting almost certainly due to your |let| stuff, so you'll probably want to investigate this...

Comment 3

[Jeff Walden [:Waldo]]

While I'm thinking about it, the output of jsDriver.pl with the -t flag for the test is:

-*- exit code 0, exit signal 5.

I'm told the expected return code in this case should be a 3.

Comment 4

[Blake Kaplan (:mrbkap) (inactive)]

Bob, Jeff, do you want to keep this bug for figuring out the jsDriver.pl problem and one of us can file a new bug on the assertion (which is unrelated)?

Comment 5

[Jeff Walden [:Waldo]]

(In reply to comment #4)
> Bob, Jeff, do you want to keep this bug for figuring out the jsDriver.pl
> problem and one of us can file a new bug on the assertion (which is unrelated)?

Filed bug 344139.

Comment 6

[Bob Clary [:bc] (inactive)]

midair:

Lets keep the bug in jsDriver.pl and the actual bug separate.

Jeff, Please file a bug on the assertion and attach your testcase there. I'll take a look at what jsDriver is doing when adding it to the test suite.

Comment 7

[Bob Clary [:bc] (inactive)]

I tried to run these on winxp but got the same results in that I didn't get an indication that the test asserted. But looking at my logs it appears the qa farm is reporting the failure on winxp only....

Testcase js1_5/Regress/regress-343713.js failed Bug Number none
[ Top of Page ]
Expected exit code 0, got 3
Testcase terminated with signal 0
Complete testcase output was:
Testcase produced no output!

I'll dig into this today as it hides errors from my review. Thanks again Jeff. 

Comment 8

[Bob Clary [:bc] (inactive)]

err, s/is reporting the failure on winxp only/is reporting the failure on win2k3 server only/

Comment 9

[Bob Clary [:bc] (inactive)]

JS_Assert calles abort. <http://lxr.mozilla.org/mozilla/source/js/src/jsutil.c#54>.

http://msdn2.microsoft.com/en-us/library/k089yyh0(d=ide).aspx claims that vc returns an exit code of 3 unless SIGABRT has been caught. The unix man pages for abort don't really specify the exit code when abort is called. I think that this isn't so much a problem with jsDriver.pl as it is that abort doesn't consistently set a non-zero exit code.

mrbkap?

Comment 10

[Bob Clary [:bc] (inactive)]

Ok. from what Waldo i showing on Linux, the exit code is by default munged to remove the signal leaving the exit code at 0 which jsDriver thinks is a pass. 

I have two questions: 

1) should I run the tests with -x to not munge the exit code and to force jsDriver.pl to report failures for signals

2) why am I not getting any exit code|signal in windows at all? (except maybe for win2k3)

Comment 11

[Bob Clary [:bc] (inactive)]

After talking it over with mrbkap, we think the proper behavior of jsDriver should be to report a failure if the exit code does not match the expected exit code _or_ if the signal is non zero. That would catch both the exit code munging and non-munging situations.

I still have the windows issue though.

Comment 12

[Bob Clary [:bc] (inactive)]

Attachment: jsDriver.pl.patch

This adds a check for $exit_signal to force failure if the signal is non-zero whether or not exit code munging has been requested. This will make sure that JS_Asserts are reported as failures.

It also adds as an output file a list of failed tests in suitable form to be fed to jsDriver.pl as an -l argument. Brendan, I didn't use the results-(.*).html -> failures-$1.txt transform as it was more difficult to handle the general case where the user specified the output file name. Instead I used (.*).html-> $1-failures.txt. For example, it will produce the following output files 

2006-07-10-21-23-31-firefox-1.8.0.5_2006070706-debug-Sophie-e4x-failures.txt
2006-07-10-21-23-31-firefox-1.8.0.5_2006070706-debug-Sophie-e4x.html

Comment 13

[Bob Clary [:bc] (inactive)]

Ok, I found the cause of the lack of exit code on Windows. In abort.c in the MSVC standard library source is this comment:

*   Multi-thread version does not raise SIGABRT -- this isn't supported
*   under multi-thread.

We (I) pass  -MD as a compiler option which is according to the compiler options for vc8:  
-MD  Creates a multithreaded DLL using MSVCRT.lib

Which is wrong in of itself since it should be using 
-MDd Creates a multithreaded DLL using MSVCRTD.lib

looking in WINNT*.mk I found

# MSVC compiler options for debug builds linked to MSVCRTD.DLL
# -MDd     - link with MSVCRTD.LIB (Dynamically-linked, multi-threaded, debug C-runtime)
# -Od      - minimal optimization
WIN_IDG_CFLAGS = -MDd -Od -Z7 

# MSVC compiler options for debug builds linked to MSVCRT.DLL
# -MD      - link with MSVCRT.LIB (Dynamically-linked, multi-threaded, debug C-runtime)
# -Od      - minimal optimization
WIN_DEBUG_CFLAGS = -MD -Od -Zi -Fd$(OBJDIR)/$(PDBFILE)

What should be the correct options for WIN_DEBUG_CFLAGS?

Comment 14

[Blake Kaplan (:mrbkap) (inactive)]

Comment on attachment 228767 [details] [diff] [review]
jsDriver.pl.patch

Seems reasonable.

Comment 15

[Bob Clary [:bc] (inactive)]

Checking in jsDriver.pl;
/cvsroot/mozilla/js/tests/jsDriver.pl,v  <--  jsDriver.pl
new revision: 1.64; previous revision: 1.63
done