Add output when compiler exits with non-zero exitcode; add compiler output to verbose

ASSIGNED
Assigned to

Status

P4
enhancement
ASSIGNED
7 years ago
7 years ago

People

(Reporter: cpeyer, Assigned: cpeyer)

Tracking

unspecified
Q1 12 - Brannan
x86
All
Bug Flags:
flashplayer-injection -
flashplayer-qrb +
flashplayer-bug -
flashplayer-triage +

Details

Attachments

(1 attachment, 4 obsolete attachments)

(Assignee)

Description

7 years ago
Created attachment 563276 [details] [diff] [review]
add error and verbose output

Add compiler output to verbose mode.

When compiler exits with non-zero exitcode, display an error message.  Note that only a message is displayed, the test is not failed.
Attachment #563276 - Flags: review?(brbaker)
Comment on attachment 563276 [details] [diff] [review]
add error and verbose output

(Call me crazy, but it seems like this patch includes a lot of stuff that is not relevant to this ticket.)

Comment 2

7 years ago
Comment on attachment 563276 [details] [diff] [review]
add error and verbose output

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

please clean up the patch
Attachment #563276 - Flags: review?(brbaker) → review-
(Assignee)

Comment 3

7 years ago
Created attachment 563437 [details] [diff] [review]
Correct patch

Forgot a . when creating patch.  Correct patch now pushed.
Attachment #563276 - Attachment is obsolete: true
Attachment #563437 - Flags: review?(brbaker)

Comment 4

7 years ago
Comment on attachment 563437 [details] [diff] [review]
Correct patch

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

I don't think this is doing what you want. I tweaked a file that I was forcing to be recompiled and the exitcode from the compilation was still 0 even though it did not compile. I duplicated the COMPILER EXITCODE logging outside of the if block, and you can see that it is returning 0 even though it fails to compile.

./runtests.py -f as3/AbcDecoder/simple.as
Tamarin tests started: 2011-09-29 13:33:31.278748
current configuration: x64-mac-tvm-release
avm version: 6579:41f0b26644a0
asc version: 5
thread count: 24
Executing 1 tests against vm: /Users/brbaker/hg/tamarin-redux/objdir/shell/avmshell
ERROR! COMPILER EXITCODE=0

[Compiler] Error #1084: Syntax error: expecting identifier before semicolon.
   as3/AbcDecoder/simple.as, Ln 46, Col 4: 
   var;
   ...^

1 error found
   FAILED! file did not compile: as3/AbcDecoder/simple.abc

FAILURES:
  as3/AbcDecoder/simple.abc : FAILED! file did not compile: as3/AbcDecoder/simple.abc
END FAILURES

::: test/util/runtestBase.py
@@ +1066,5 @@
>          try:
>              (f,err,exitcode) = self.run_pipe('%s %s' % (cmd,as_file),
>                                               outputCalls=outputCalls)
> +            if exitcode != 0:
> +                outputCalls.append((self.js_print, ('ERROR! COMPILER EXITCODE='+exitcode,)))

Can't combine string and ints here, need to do:

'ERROR! COMPILER EXITCODE=%s'%exitcode
Attachment #563437 - Flags: review?(brbaker) → review-

Comment 5

7 years ago
(In reply to Brent Baker from comment #4)
> ::: test/util/runtestBase.py
> @@ +1066,5 @@
> >          try:
> >              (f,err,exitcode) = self.run_pipe('%s %s' % (cmd,as_file),
> >                                               outputCalls=outputCalls)
> > +            if exitcode != 0:
> > +                outputCalls.append((self.js_print, ('ERROR! COMPILER EXITCODE='+exitcode,)))
> 
> Can't combine string and ints here, need to do:
> 
> 'ERROR! COMPILER EXITCODE=%s'%exitcode

Actually I guess it should be:

outputCalls.append((self.js_print, ('ERROR! COMPILER EXITCODE=%i'%exitcode,)))
(Assignee)

Comment 6

7 years ago
Created attachment 563457 [details] [diff] [review]
fix str + int issue
Attachment #563437 - Attachment is obsolete: true
Attachment #563457 - Flags: review?(brbaker)

Comment 7

7 years ago
Comment on attachment 563457 [details] [diff] [review]
fix str + int issue

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

Still marking as r- since asc is returning a 0 exitcode even when it fails to compile, so this is NOT doing what you expect
Attachment #563457 - Flags: review?(brbaker) → review-
(Assignee)

Comment 8

7 years ago
Created attachment 563466 [details] [diff] [review]
check for non-zero exitcode - add comment explaining code

Unfortunately asc has a zero exitcode even when there are errors, so the exitcode checking only applies to falcon.

Verbose code still works for both compilers.
Attachment #563457 - Attachment is obsolete: true
Attachment #563466 - Flags: review?(brbaker)

Updated

7 years ago
Attachment #563466 - Flags: review?(brbaker) → review+

Updated

7 years ago
Flags: flashplayer-triage+
Flags: flashplayer-qrb+
Flags: flashplayer-injection-
Flags: flashplayer-bug-
Target Milestone: --- → Q1 12 - Brannan

Updated

7 years ago
Priority: -- → P4
(Assignee)

Comment 9

7 years ago
Created attachment 566658 [details] [diff] [review]
instead of printing warning, fail the test

Improvement on previous patch.  Treat non-zero exitcode as a failure, add support to set it as an expected failure.
Attachment #563466 - Attachment is obsolete: true
Attachment #566658 - Flags: review?(brbaker)

Updated

7 years ago
Attachment #566658 - Flags: review?(brbaker) → review+
You need to log in before you can comment on or make changes to this bug.