Bug 120722 - run-javascriptcore-tests should run-fast-jsc as well
Summary: run-javascriptcore-tests should run-fast-jsc as well
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Filip Pizlo
URL:
Keywords:
Depends on:
Blocks: 120696
  Show dependency treegraph
 
Reported: 2013-09-04 20:55 PDT by Filip Pizlo
Modified: 2013-09-05 08:08 PDT (History)
9 users (show)

See Also:


Attachments
the patch (6.56 KB, patch)
2013-09-04 20:58 PDT, Filip Pizlo
no flags Details | Formatted Diff | Diff
the patch (8.76 KB, patch)
2013-09-04 21:30 PDT, Filip Pizlo
ggaren: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Filip Pizlo 2013-09-04 20:55:13 PDT
Patch forthcoming.
Comment 1 Filip Pizlo 2013-09-04 20:58:47 PDT
Created attachment 210543 [details]
the patch
Comment 2 Filip Pizlo 2013-09-04 21:00:36 PDT
What the output looks like right now:


.... bunch of stuff

** The following fast/js test failures have been introduced:
	fast/js/exception-expression-offset
	fast/js/Object-defineProperty

Results for Mozilla tests:
    0 regressions found.
    0 tests fixed.
    OK.

Results for fast/js tests:
    2 failures found.
    0 crashes found.
Comment 3 Filip Pizlo 2013-09-04 21:30:44 PDT
Created attachment 210545 [details]
the patch
Comment 4 Geoffrey Garen 2013-09-04 21:33:01 PDT
Comment on attachment 210545 [details]
the patch

r=me

Nice.

Why do we need jsc-test-list? Do only a subset of tests work in this way?
Comment 5 Filip Pizlo 2013-09-04 21:35:19 PDT
(In reply to comment #4)
> (From update of attachment 210545 [details])
> r=me
> 
> Nice.
> 
> Why do we need jsc-test-list? Do only a subset of tests work in this way?

Good question.

The short answer is that only a subset of fast/js tests work in the jsc shell because a lot of them depend on DOM behavior.  Some of them are specifically testing the JS side of DOM behavior.

The long answer is that we should just use directory structure to dictate this.  Any test that isn't in jsc-test-list should just be in a different directory, like I don't know, fast/js/dom

RS=you to do such test motion to fast/js/dom for those tests that aren't pure JS?
Comment 6 Filip Pizlo 2013-09-04 21:43:21 PDT
Landed in http://trac.webkit.org/changeset/155092
Comment 7 Geoffrey Garen 2013-09-04 21:43:49 PDT
Sounds good, rs = me.
Comment 8 Csaba Osztrogonác 2013-09-04 23:56:34 PDT
(In reply to comment #6)
> Landed in http://trac.webkit.org/changeset/155092

Unfortunately it broke run-javascriptcore-tests on linux bots
at least because of the following reasons:
- slurp isn't installed on the linux bots (EFL/GTK/Qt)
- function and let aren't valid keyword in dash
  (dash is the default shell on Ubuntu!)
Comment 9 Filip Pizlo 2013-09-04 23:57:28 PDT
(In reply to comment #8)
> (In reply to comment #6)
> > Landed in http://trac.webkit.org/changeset/155092
> 
> Unfortunately it broke run-javascriptcore-tests on linux bots
> at least because of the following reasons:
> - slurp isn't installed on the linux bots (EFL/GTK/Qt)
> - function and let aren't valid keyword in dash
>   (dash is the default shell on Ubuntu!)

That's unfortunate, now can you fix it?
Comment 10 WebKit Commit Bot 2013-09-04 23:58:07 PDT
Re-opened since this is blocked by bug 120756
Comment 11 Filip Pizlo 2013-09-05 00:00:23 PDT
(In reply to comment #9)
> (In reply to comment #8)
> > (In reply to comment #6)
> > > Landed in http://trac.webkit.org/changeset/155092
> > 
> > Unfortunately it broke run-javascriptcore-tests on linux bots
> > at least because of the following reasons:
> > - slurp isn't installed on the linux bots (EFL/GTK/Qt)
> > - function and let aren't valid keyword in dash
> >   (dash is the default shell on Ubuntu!)
> 
> That's unfortunate, now can you fix it?

(I hope that I am being rather unambiguous that rolling this out because you're too lazy to help look for the Linux-specific workaround would be irresponsible.)
Comment 12 Csaba Osztrogonác 2013-09-05 00:01:02 PDT
(In reply to comment #9)
> (In reply to comment #8)
> > (In reply to comment #6)
> > > Landed in http://trac.webkit.org/changeset/155092
> > 
> > Unfortunately it broke run-javascriptcore-tests on linux bots
> > at least because of the following reasons:
> > - slurp isn't installed on the linux bots (EFL/GTK/Qt)
> > - function and let aren't valid keyword in dash
> >   (dash is the default shell on Ubuntu!)
> 
> That's unfortunate, now can you fix it?

Of course, I can, but not now, only ~3-4 hours later.
Comment 13 Filip Pizlo 2013-09-05 00:02:13 PDT
(In reply to comment #12)
> (In reply to comment #9)
> > (In reply to comment #8)
> > > (In reply to comment #6)
> > > > Landed in http://trac.webkit.org/changeset/155092
> > > 
> > > Unfortunately it broke run-javascriptcore-tests on linux bots
> > > at least because of the following reasons:
> > > - slurp isn't installed on the linux bots (EFL/GTK/Qt)
> > > - function and let aren't valid keyword in dash
> > >   (dash is the default shell on Ubuntu!)
> > 
> > That's unfortunate, now can you fix it?
> 
> Of course, I can, but not now, only ~3-4 hours later.

Then leave it in and I'll make it so that Linux doesn't run the new tests.
Comment 14 Filip Pizlo 2013-09-05 00:03:23 PDT
(In reply to comment #8)
> (In reply to comment #6)
> > Landed in http://trac.webkit.org/changeset/155092
> 
> Unfortunately it broke run-javascriptcore-tests on linux bots
> at least because of the following reasons:
> - slurp isn't installed on the linux bots (EFL/GTK/Qt)

I can fix this, give me a sec.

> - function and let aren't valid keyword in dash
>   (dash is the default shell on Ubuntu!)

Three choices:

1) You can fix the shell script to use whatever language "dash" uses.

2) Install a compliant shell.

3) Disable the new tests on Linux.

I'll do (3).  If you want to implement one of the other fixes, then that's fine.
Comment 15 Csaba Osztrogonác 2013-09-05 00:06:24 PDT
(In reply to comment #11)
> (I hope that I am being rather unambiguous that rolling this out because you're too lazy to help look for the Linux-specific workaround would be irresponsible.)

Lazy? What's wrong with you men? Please don't expect me 
to fix your regression instead of you _immediately_ ...

I do always willingly help fixing things ... I'd like to help you ... 
after I go to the office ... But considering me lazy wasn't so friendly ...
Comment 16 Filip Pizlo 2013-09-05 00:13:16 PDT
(In reply to comment #15)
> (In reply to comment #11)
> > (I hope that I am being rather unambiguous that rolling this out because you're too lazy to help look for the Linux-specific workaround would be irresponsible.)
> 
> Lazy? What's wrong with you men? Please don't expect me 
> to fix your regression instead of you _immediately_ ...
> 
> I do always willingly help fixing things ... I'd like to help you ... 
> after I go to the office ... But considering me lazy wasn't so friendly ...

I'm sorry.  But next time you can give me a bit more time before starting a rollout.
Comment 17 Csaba Osztrogonác 2013-09-05 07:56:15 PDT
(In reply to comment #16)
> (In reply to comment #15)
> > (In reply to comment #11)
> > > (I hope that I am being rather unambiguous that rolling this out because you're too lazy to help look for the Linux-specific workaround would be irresponsible.)
> > 
> > Lazy? What's wrong with you men? Please don't expect me 
> > to fix your regression instead of you _immediately_ ...
> > 
> > I do always willingly help fixing things ... I'd like to help you ... 
> > after I go to the office ... But considering me lazy wasn't so friendly ...
> 
> I'm sorry.  But next time you can give me a bit more time before starting a rollout.

Not problem. I'm sorry too for the missing communication. 

But make it clear, you landed the patch ~2 hours previously, you could have
notice that it broke everything. ( It was a little bit similar to
http://webkitmemes.tumblr.com/post/18264800090/cool-developers-dont-look-at-the-build-bots )
Additionally I tried to contact you on #webkit, but you were offline. It's
absolutely reasonable at midnight and I didn't expect you to fix this bug
instead of sleeping. ;) I simple thought you left.

Then I asked the webkitbot to create a rollout patch and started to write a
similar comment to the bug: "I don't have time for debugging now, so I'm going
to roll it out to make bots happier and will check it a little bit later after
I go to the office in ~3-4 hours" But the bugzilla refused the comment,
because you already closed the bug and added that disparagement comment.

But please forget this misunderstanding situation and let's try 
to continue making WebKit better together without fighting.

Thanks for these fixes:
- https://trac.webkit.org/changeset/155101
- https://trac.webkit.org/changeset/155103

I tried to be constructive and fixed the run-fast-jsc script for dash:
- https://bugs.webkit.org/show_bug.cgi?id=120759

filed a bug for Windows failures:
- https://bugs.webkit.org/show_bug.cgi?id=120765

and made master.cfg to understand the new output:
- https://bugs.webkit.org/show_bug.cgi?id=120766
Comment 18 Filip Pizlo 2013-09-05 08:08:09 PDT
(In reply to comment #17)
> (In reply to comment #16)
> > (In reply to comment #15)
> > > (In reply to comment #11)
> > > > (I hope that I am being rather unambiguous that rolling this out because you're too lazy to help look for the Linux-specific workaround would be irresponsible.)
> > > 
> > > Lazy? What's wrong with you men? Please don't expect me 
> > > to fix your regression instead of you _immediately_ ...
> > > 
> > > I do always willingly help fixing things ... I'd like to help you ... 
> > > after I go to the office ... But considering me lazy wasn't so friendly ...
> > 
> > I'm sorry.  But next time you can give me a bit more time before starting a rollout.
> 
> Not problem. I'm sorry too for the missing communication. 
> 
> But make it clear, you landed the patch ~2 hours previously, you could have
> notice that it broke everything. ( It was a little bit similar to
> http://webkitmemes.tumblr.com/post/18264800090/cool-developers-dont-look-at-the-build-bots )
> Additionally I tried to contact you on #webkit, but you were offline. It's
> absolutely reasonable at midnight and I didn't expect you to fix this bug
> instead of sleeping. ;) I simple thought you left.

It takes ~1 hour for the Linux test bots to cycle.  So yeah, I noticed it with a delay.

> 
> Then I asked the webkitbot to create a rollout patch and started to write a
> similar comment to the bug: "I don't have time for debugging now, so I'm going
> to roll it out to make bots happier and will check it a little bit later after
> I go to the office in ~3-4 hours" But the bugzilla refused the comment,
> because you already closed the bug and added that disparagement comment.
> 
> But please forget this misunderstanding situation and let's try 
> to continue making WebKit better together without fighting.
> 
> Thanks for these fixes:
> - https://trac.webkit.org/changeset/155101
> - https://trac.webkit.org/changeset/155103
> 
> I tried to be constructive and fixed the run-fast-jsc script for dash:
> - https://bugs.webkit.org/show_bug.cgi?id=120759
> 
> filed a bug for Windows failures:
> - https://bugs.webkit.org/show_bug.cgi?id=120765
> 
> and made master.cfg to understand the new output:
> - https://bugs.webkit.org/show_bug.cgi?id=120766

Awesome, thanks for those!