Re: [webkit-dev] run-bindings-tests

2011-09-12 Thread Maciej Stachowiak

On Sep 8, 2011, at 12:25 PM, Darin Adler wrote:

 On Sep 8, 2011, at 11:49 AM, Alexey Proskuryakov wrote:
 
 As discussed on IRC, I do not think that bots should run this test at all. 
 It has a non-trivial maintenance cost, but provides very little benefit. 
 Even the time spent by multiple engineers on IRC today discussing bot 
 complaints is likely more than the test could save in the lifetime of the 
 project, at my guesstimate.
 
 I find the bindings tests quite helpful. Because the perl script is so hard 
 to read, it’s the changes in bindings script test results that I look at when 
 reviewing changes to the bindings scripts. The fact that the results are 
 checked in helps me review patches.
 
 It would be better to plug them in to the testing machinery in a better way. 
 I don’t think contributors should have to learn how to run different types of 
 commands.

Notwithstanding all the later discussion, I think run-bindings-tests would 
still be more effective as a build step that updates a source file rather than 
a test step. Recompiling after changing the bindings generator would then 
regenerate this file, and the diffs would be present in uploaded patches (as 
well as being obtainable to developers working locally by using svn diff or git 
diff respectively). That way, it's much harder to do it wrong and cause bot 
redness downstream. It's possible that this way you could cause bindings 
changes unintentionally, but presumably you and your reviewer will spot these 
when looking at the patch. It seems to me we shouldn't require an extra manual 
step to say I really meant to change the text of the generated bindings.

Regards,
Maciej

___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev


Re: [webkit-dev] run-bindings-tests

2011-09-12 Thread Darin Adler
On Sep 12, 2011, at 4:23 PM, Maciej Stachowiak wrote:

 Notwithstanding all the later discussion, I think run-bindings-tests would 
 still be more effective as a build step that updates a source file rather 
 than a test step.

I see, a build step that updates a checked-in source file. Sounds like a great 
idea to me. I did not see that proposal earlier!

-- Darin

___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev


Re: [webkit-dev] run-bindings-tests

2011-09-09 Thread Oliver Hunt

On Sep 8, 2011, at 7:21 PM, Alexey Proskuryakov wrote:

 
 08.09.2011, в 12:25, Darin Adler написал(а):
 
 I find the bindings tests quite helpful. Because the perl script is so hard 
 to read, it’s the changes in bindings script test results that I look at 
 when reviewing changes to the bindings scripts. The fact that the results 
 are checked in helps me review patches.
 
 OK, then they are valuable indeed.
 
 However, I still feel that there is a disconnect between the desired effect 
 (provide a diff in a patch for review) and the implementation (tests that can 
 pass or fail). This also puts the burden of maintaining the results on people 
 who needn't care about them - for example, Oliver's patch clearly didn't need 
 someone look over generated code changes.

I think the argument is that it _did_ need someone - the reviewer didn't have 
any nice way to see the difference in output that would have been visible had 
i included updates to the expected output.

My problem with the test is that it isn't run as part of run-webkit-tests 
(which is what we say you must run), the test output is fairly awful, and the 
test script doesn't support --help, or --reset (it turns out it does have an 
equivalent to --reset, but why use a different argument in one tool from what 
we use in the main one?)

--Oliver

 
 I'm not sure what the better solution would be though. Perhaps a bot could 
 provide a diff of DerivedSources for any patch that touches code generators, 
 but I'm not volunteering to implement one :-)
 
 - WBR, Alexey Proskuryakov
 
 ___
 webkit-dev mailing list
 webkit-dev@lists.webkit.org
 http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev

___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev


Re: [webkit-dev] run-bindings-tests

2011-09-09 Thread Adam Barth
2011/9/8 Oliver Hunt oli...@apple.com:
 On Sep 8, 2011, at 7:21 PM, Alexey Proskuryakov wrote:
 08.09.2011, в 12:25, Darin Adler написал(а):
 I find the bindings tests quite helpful. Because the perl script is so hard 
 to read, it’s the changes in bindings script test results that I look at 
 when reviewing changes to the bindings scripts. The fact that the results 
 are checked in helps me review patches.

 OK, then they are valuable indeed.

 However, I still feel that there is a disconnect between the desired effect 
 (provide a diff in a patch for review) and the implementation (tests that 
 can pass or fail). This also puts the burden of maintaining the results on 
 people who needn't care about them - for example, Oliver's patch clearly 
 didn't need someone look over generated code changes.

 I think the argument is that it _did_ need someone - the reviewer didn't 
 have any nice way to see the difference in output that would have been 
 visible had i included updates to the expected output.

 My problem with the test is that it isn't run as part of run-webkit-tests 
 (which is what we say you must run), the test output is fairly awful, and the 
 test script doesn't support --help, or --reset (it turns out it does have an 
 equivalent to --reset, but why use a different argument in one tool from what 
 we use in the main one?)

Those all sound like very fixable issues.  I'm sorry I picked the
wrong flag name.  I was trying to copy the name of the flag used by
run-webkit-tests, but I must have screwed it up somehow.

If you'd be willing to file bugs about the improvements you'd like to
see in the test, I'll be happy to make them.  I'm not quite up to the
task of making run-webkit-tests run all the various tests, but
hopefully someone will volunteer to make that happen.

Adam


 I'm not sure what the better solution would be though. Perhaps a bot could 
 provide a diff of DerivedSources for any patch that touches code generators, 
 but I'm not volunteering to implement one :-)
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev


Re: [webkit-dev] run-bindings-tests

2011-09-09 Thread Eric Seidel
On Thu, Sep 8, 2011 at 12:44 PM, Darin Adler da...@apple.com wrote:
 On Sep 8, 2011, at 12:29 PM, Eric Seidel wrote:

 I'm happy to write a run-all-tests script which runs all known tests that 
 platform can handle. :)

 I think run-webkit-tests should be this. We can come up with a new name for 
 the “just run the tests in the LayoutTests directory” tool.

I would like that too.  But there are going to be stages to get here.
If we did this today, bots would break if nothing else.

I'll see about adding a run-all-tests script soon and we can work to
have it replace run-webkit-tests.  (new-new-run-webkit-tests anyone?)
;)

 A bigger problem is the different way that all the various tests indicate 
 tests have run, succeeded, or failed. If you try to run all of these it’s not 
 trivial to figure out what happened.

    -- Darin


___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev


Re: [webkit-dev] run-bindings-tests

2011-09-09 Thread Adam Roben
On Sep 9, 2011, at 10:52 AM, Eric Seidel wrote:

 On Thu, Sep 8, 2011 at 12:44 PM, Darin Adler da...@apple.com wrote:
 On Sep 8, 2011, at 12:29 PM, Eric Seidel wrote:
 
 I'm happy to write a run-all-tests script which runs all known tests that 
 platform can handle. :)
 
 I think run-webkit-tests should be this. We can come up with a new name for 
 the “just run the tests in the LayoutTests directory” tool.
 
 I would like that too.  But there are going to be stages to get here.
 If we did this today, bots would break if nothing else.
 
 I'll see about adding a run-all-tests script soon and we can work to
 have it replace run-webkit-tests.  (new-new-run-webkit-tests anyone?)

An alternate path to success would be:

1) Decide what to call the script that just runs tests from LayoutTests
2) Rename run-webkit-tests to that new name and add a new script called 
run-webkit-tests that just calls the renamed script
3) Slowly make run-webkit-tests call out to other test-running scripts

-Adam

___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev


Re: [webkit-dev] run-bindings-tests

2011-09-09 Thread Adam Roben
On Sep 9, 2011, at 10:59 AM, Adam Roben wrote:

 On Sep 9, 2011, at 10:52 AM, Eric Seidel wrote:
 
 On Thu, Sep 8, 2011 at 12:44 PM, Darin Adler da...@apple.com wrote:
 On Sep 8, 2011, at 12:29 PM, Eric Seidel wrote:
 
 I'm happy to write a run-all-tests script which runs all known tests that 
 platform can handle. :)
 
 I think run-webkit-tests should be this. We can come up with a new name for 
 the “just run the tests in the LayoutTests directory” tool.
 
 I would like that too.  But there are going to be stages to get here.
 If we did this today, bots would break if nothing else.
 
 I'll see about adding a run-all-tests script soon and we can work to
 have it replace run-webkit-tests.  (new-new-run-webkit-tests anyone?)
 
 An alternate path to success would be:
 
 1) Decide what to call the script that just runs tests from LayoutTests
 2) Rename run-webkit-tests to that new name and add a new script called 
 run-webkit-tests that just calls the renamed script
 3) Slowly make run-webkit-tests call out to other test-running scripts

I of course forgot:

2.5) Change the bots to call the renamed script

-Adam

___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev


Re: [webkit-dev] run-bindings-tests

2011-09-09 Thread Adam Barth
run-layout-tests?
On Sep 9, 2011 8:04 AM, Adam Roben aro...@apple.com wrote:
 On Sep 9, 2011, at 10:59 AM, Adam Roben wrote:

 On Sep 9, 2011, at 10:52 AM, Eric Seidel wrote:

 On Thu, Sep 8, 2011 at 12:44 PM, Darin Adler da...@apple.com wrote:
 On Sep 8, 2011, at 12:29 PM, Eric Seidel wrote:

 I'm happy to write a run-all-tests script which runs all known tests
that platform can handle. :)

 I think run-webkit-tests should be this. We can come up with a new name
for the “just run the tests in the LayoutTests directory” tool.

 I would like that too. But there are going to be stages to get here.
 If we did this today, bots would break if nothing else.

 I'll see about adding a run-all-tests script soon and we can work to
 have it replace run-webkit-tests. (new-new-run-webkit-tests anyone?)

 An alternate path to success would be:

 1) Decide what to call the script that just runs tests from LayoutTests
 2) Rename run-webkit-tests to that new name and add a new script called
run-webkit-tests that just calls the renamed script
 3) Slowly make run-webkit-tests call out to other test-running scripts

 I of course forgot:

 2.5) Change the bots to call the renamed script

 -Adam

 ___
 webkit-dev mailing list
 webkit-dev@lists.webkit.org
 http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev


Re: [webkit-dev] run-bindings-tests

2011-09-09 Thread Jarred Nicholls
On Fri, Sep 9, 2011 at 12:07 PM, Adam Barth aba...@webkit.org wrote:

 run-layout-tests?

too obvious.  as an ode to Paul, I propose
run-the-mother-effing-layout-tests

http://mothereffinghsl.com/

;)


  On Sep 9, 2011 8:04 AM, Adam Roben aro...@apple.com wrote:
  On Sep 9, 2011, at 10:59 AM, Adam Roben wrote:
 
  On Sep 9, 2011, at 10:52 AM, Eric Seidel wrote:
 
  On Thu, Sep 8, 2011 at 12:44 PM, Darin Adler da...@apple.com wrote:
  On Sep 8, 2011, at 12:29 PM, Eric Seidel wrote:
 
  I'm happy to write a run-all-tests script which runs all known tests
 that platform can handle. :)
 
  I think run-webkit-tests should be this. We can come up with a new
 name for the “just run the tests in the LayoutTests directory” tool.
 
  I would like that too. But there are going to be stages to get here.
  If we did this today, bots would break if nothing else.
 
  I'll see about adding a run-all-tests script soon and we can work to
  have it replace run-webkit-tests. (new-new-run-webkit-tests anyone?)
 
  An alternate path to success would be:
 
  1) Decide what to call the script that just runs tests from LayoutTests
  2) Rename run-webkit-tests to that new name and add a new script called
 run-webkit-tests that just calls the renamed script
  3) Slowly make run-webkit-tests call out to other test-running scripts
 
  I of course forgot:
 
  2.5) Change the bots to call the renamed script
 
  -Adam
 
  ___
  webkit-dev mailing list
  webkit-dev@lists.webkit.org
  http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev

 ___
 webkit-dev mailing list
 webkit-dev@lists.webkit.org
 http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev




-- 


*Sencha*
Jarred Nicholls, Senior Software Architect
@jarrednicholls
http://twitter.com/jarrednicholls
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev


Re: [webkit-dev] run-bindings-tests

2011-09-09 Thread Darin Adler
On Sep 9, 2011, at 9:07 AM, Adam Barth wrote:

 run-layout-tests?

Sorry in advance for bikeshed'ing this:

That would be a good name if we thought LayoutTests was the right name for our 
main regression test suite. Since I think it’s not, I would love to figure out 
that new name and have the script reflect the new name even before we rename 
the directory.

I also think it’s good to have a WebKit-specific or specific-enough word in 
script names when possible so you can have the scripts in your path even when 
not working on WebKit. That’s why run-webkit-tests has the word WebKit in it, 
and run-safari does not.

-- Darin

___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev


Re: [webkit-dev] run-bindings-tests

2011-09-09 Thread Geoffrey Garen
 I also think it’s good to have a WebKit-specific or specific-enough word in 
 script names when possible so you can have the scripts in your path even when 
 not working on WebKit. That’s why run-webkit-tests has the word WebKit in it, 
 and run-safari does not.

I'd suggest one script -- run-webkit-tests -- with flags for including or 
excluding specific testing (layout tests, unit tests, bindings tests, etc.). 
That way, there's only one name to remember, you get a sensible default, and 
you can always use --help to figure special subtest settings as necessary. 

(I think a little bike shedding is OK here, since this is a tool we all use 
every day.)

Geoff___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev


Re: [webkit-dev] run-bindings-tests

2011-09-09 Thread Ryosuke Niwa
On Fri, Sep 9, 2011 at 7:59 AM, Adam Roben aro...@apple.com wrote:

 An alternate path to success would be:

 1) Decide what to call the script that just runs tests from LayoutTests
 2) Rename run-webkit-tests to that new name and add a new script called
 run-webkit-tests that just calls the renamed script
 3) Slowly make run-webkit-tests call out to other test-running scripts


Just as a remainder, step 2 will require modifying various scripts and, most
importantly, bot configurations that refer to run-webkit-tests or
new-run-webkit-tests.

- Ryosuke
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev


Re: [webkit-dev] run-bindings-tests

2011-09-09 Thread Dirk Pranke
On Fri, Sep 9, 2011 at 10:17 AM, Geoffrey Garen gga...@apple.com wrote:
 I also think it’s good to have a WebKit-specific or specific-enough word in
 script names when possible so you can have the scripts in your path even
 when not working on WebKit. That’s why run-webkit-tests has the word WebKit
 in it, and run-safari does not.

 I'd suggest one script -- run-webkit-tests -- with flags for including or
 excluding specific testing (layout tests, unit tests, bindings tests, etc.).
 That way, there's only one name to remember, you get a sensible default, and
 you can always use --help to figure special subtest settings as necessary.
 (I think a little bike shedding is OK here, since this is a tool we all use
 every day.)

Note that the mechanisms for running each sub-suite may vary
significantly, so practically the top-level script may just end up
shelling out to subscripts anyway. That said, I agree that the
top-level script should have the characteristics you describe.

-- Dirk
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev


[webkit-dev] run-bindings-tests

2011-09-08 Thread Eric Seidel
FYI:  As many of you already know, the build.webkit.org bots run
run-bindings-tests on (almost) all platforms.

They've been running (mostly w/o incident) on the bots since 6/20:
http://trac.webkit.org/changeset/89267


These just make sure that our generated bindings look sane, by
comparing the generated results against checked-in baselines.

run-bindings-tests makes it easier to make cross-platform bindings
changes w/o needing a Gtk/Qt/V8/etc. port of WebKit.

If you're changing binding generation you should be aware of this script.

Thanks!

-eric
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev


Re: [webkit-dev] run-bindings-tests

2011-09-08 Thread Alexey Proskuryakov

08.09.2011, в 11:32, Eric Seidel написал(а):

 FYI:  As many of you already know, the build.webkit.org bots run
 run-bindings-tests on (almost) all platforms.
 
 They've been running (mostly w/o incident) on the bots since 6/20:
 http://trac.webkit.org/changeset/89267
 
 
 These just make sure that our generated bindings look sane, by
 comparing the generated results against checked-in baselines.
 
 run-bindings-tests makes it easier to make cross-platform bindings
 changes w/o needing a Gtk/Qt/V8/etc. port of WebKit.
 
 If you're changing binding generation you should be aware of this script.

As discussed on IRC, I do not think that bots should run this test at all. It 
has a non-trivial maintenance cost, but provides very little benefit. Even the 
time spent by multiple engineers on IRC today discussing bot complaints is 
likely more than the test could save in the lifetime of the project, at my 
guesstimate.

A test like this is almost like keeping a separate text file with a number of 
space characters in WebKit sources, and chastising anyone who fails to update 
this text file with their commit. Why would we care about the number of spaces, 
or about the exact look of generated code?

Specifically, this is today's failure: 
http://build.webkit.org/builders/SnowLeopard%20Intel%20Release%20%28Tests%29/builds/32923/steps/bindings-generation-tests/logs/stdio
 - a test that complains about such changes doesn't test for the right thing.

A script like this might be useful to run locally when making bindings changes 
if in doubt, comparing before and after results. There is no need to check 
in most recent results for this though. I'm not sure if this gives you more 
than manually copying DerivedSources directory and diffing new derived sources 
to that, but if someone finds a little automation valuable, then why not.

- WBR, Alexey Proskuryakov

___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev


Re: [webkit-dev] run-bindings-tests

2011-09-08 Thread James Robinson
On Thu, Sep 8, 2011 at 11:49 AM, Alexey Proskuryakov a...@webkit.org wrote:


 08.09.2011, в 11:32, Eric Seidel написал(а):

  FYI:  As many of you already know, the build.webkit.org bots run
  run-bindings-tests on (almost) all platforms.
 
  They've been running (mostly w/o incident) on the bots since 6/20:
  http://trac.webkit.org/changeset/89267
 
 
  These just make sure that our generated bindings look sane, by
  comparing the generated results against checked-in baselines.
 
  run-bindings-tests makes it easier to make cross-platform bindings
  changes w/o needing a Gtk/Qt/V8/etc. port of WebKit.
 
  If you're changing binding generation you should be aware of this script.

 As discussed on IRC, I do not think that bots should run this test at all.
 It has a non-trivial maintenance cost, but provides very little benefit.
 Even the time spent by multiple engineers on IRC today discussing bot
 complaints is likely more than the test could save in the lifetime of the
 project, at my guesstimate.

 A test like this is almost like keeping a separate text file with a number
 of space characters in WebKit sources, and chastising anyone who fails to
 update this text file with their commit. Why would we care about the number
 of spaces, or about the exact look of generated code?

 Specifically, this is today's failure: 
 http://build.webkit.org/builders/SnowLeopard%20Intel%20Release%20%28Tests%29/builds/32923/steps/bindings-generation-tests/logs/stdio
 - a test that complains about such changes doesn't test for the right thing.

 A script like this might be useful to run locally when making bindings
 changes if in doubt, comparing before and after results. There is no
 need to check in most recent results for this though. I'm not sure if this
 gives you more than manually copying DerivedSources directory and diffing
 new derived sources to that, but if someone finds a little automation
 valuable, then why not.



We used to not run these tests on the bots.  This meant that people would
change the bindings code and not update the expected results, so the
expected results were always massively out of date.  This meant when
patching the bindings scripts you could not rely on run-bindings-tests at
all, because the expectations were already broken before you made any
changes!  This it not theoretical, it happened to be multiple times and I
know I'm not the only one.

The real problem here is that people check in without looking at the bots
and then do not respond when the bots go red.  That's a people problem.

- James



 - WBR, Alexey Proskuryakov

 ___
 webkit-dev mailing list
 webkit-dev@lists.webkit.org
 http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev

___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev


Re: [webkit-dev] run-bindings-tests

2011-09-08 Thread Eric Seidel
I'm not currently working on bindings, so I don't have very strong
opinions for or against the script.

I added it to the bots back in June so that the results would stop breaking.


It's possible such script would be more useful w/o checked in results, unclear.

I will point out that Darin Adler, was at one point at least one happy
customer of this script:
https://bugs.webkit.org/show_bug.cgi?id=62880#c5

-eric

On Thu, Sep 8, 2011 at 11:49 AM, Alexey Proskuryakov a...@webkit.org wrote:

 08.09.2011, в 11:32, Eric Seidel написал(а):

 FYI:  As many of you already know, the build.webkit.org bots run
 run-bindings-tests on (almost) all platforms.

 They've been running (mostly w/o incident) on the bots since 6/20:
 http://trac.webkit.org/changeset/89267


 These just make sure that our generated bindings look sane, by
 comparing the generated results against checked-in baselines.

 run-bindings-tests makes it easier to make cross-platform bindings
 changes w/o needing a Gtk/Qt/V8/etc. port of WebKit.

 If you're changing binding generation you should be aware of this script.

 As discussed on IRC, I do not think that bots should run this test at all. It 
 has a non-trivial maintenance cost, but provides very little benefit. Even 
 the time spent by multiple engineers on IRC today discussing bot complaints 
 is likely more than the test could save in the lifetime of the project, at my 
 guesstimate.

 A test like this is almost like keeping a separate text file with a number of 
 space characters in WebKit sources, and chastising anyone who fails to update 
 this text file with their commit. Why would we care about the number of 
 spaces, or about the exact look of generated code?

 Specifically, this is today's failure: 
 http://build.webkit.org/builders/SnowLeopard%20Intel%20Release%20%28Tests%29/builds/32923/steps/bindings-generation-tests/logs/stdio
  - a test that complains about such changes doesn't test for the right thing.

 A script like this might be useful to run locally when making bindings 
 changes if in doubt, comparing before and after results. There is no need 
 to check in most recent results for this though. I'm not sure if this gives 
 you more than manually copying DerivedSources directory and diffing new 
 derived sources to that, but if someone finds a little automation valuable, 
 then why not.

 - WBR, Alexey Proskuryakov


___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev


Re: [webkit-dev] run-bindings-tests

2011-09-08 Thread Oliver Hunt

On Sep 8, 2011, at 11:55 AM, James Robinson wrote:
 
 We used to not run these tests on the bots.  This meant that people would 
 change the bindings code and not update the expected results, so the expected 
 results were always massively out of date.  This meant when patching the 
 bindings scripts you could not rely on run-bindings-tests at all, because the 
 expectations were already broken before you made any changes!  This it not 
 theoretical, it happened to be multiple times and I know I'm not the only one.
 
 The real problem here is that people check in without looking at the bots and 
 then do not respond when the bots go red.  That's a people problem.

The real problem is that we have a test suite: run-webkit-tests, that everyone 
runs (it's even mentioned at step 4 on 
http://www.webkit.org/coding/contributing.html , which is apparently not 
running all the tests.

I run-webkit-tests before i commit (new-run-webkit-tests has ensured that any 
prior complains about time taken no longer exist, so kudos to those folk \o/ ), 
and yet I end up breaking the build in a way that would show up locally if the 
tests were simply run.  run-webkit-tests should run all of the webkit tests -- 
not some subset, all of them.  If failing a cross platform test can turn the 
bots red, then that test should be covered by run-webkit-tests.

The other problem is people check in without looking at the bots.  I do try 
to watch the bots, but the time between me landing and the bots actually going 
red can literally be hours.  Of course i'm away from irc/email whatever when I 
land a patch at 4pm and it turns the bots red at 11pm.

I appreciate this isn't as much of a problem for people who don't work on code 
for which all changes heralds a world rebuild and effect (apparently) every 
single test that exists in the repository, but it's certainly frustrating for 
those of us who do.

(This is ignoring the overly aggressive rollouts of large patches the break 
only single platforms due to platform specific code that is difficult for 
anyone outside of that platform to fix)

--Oliver

 - James
 
 
 
 - WBR, Alexey Proskuryakov
 
 ___
 webkit-dev mailing list
 webkit-dev@lists.webkit.org
 http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
 
 ___
 webkit-dev mailing list
 webkit-dev@lists.webkit.org
 http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev

___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev


Re: [webkit-dev] run-bindings-tests

2011-09-08 Thread Darin Adler
On Sep 8, 2011, at 11:49 AM, Alexey Proskuryakov wrote:

 As discussed on IRC, I do not think that bots should run this test at all. It 
 has a non-trivial maintenance cost, but provides very little benefit. Even 
 the time spent by multiple engineers on IRC today discussing bot complaints 
 is likely more than the test could save in the lifetime of the project, at my 
 guesstimate.

I find the bindings tests quite helpful. Because the perl script is so hard to 
read, it’s the changes in bindings script test results that I look at when 
reviewing changes to the bindings scripts. The fact that the results are 
checked in helps me review patches.

It would be better to plug them in to the testing machinery in a better way. I 
don’t think contributors should have to learn how to run different types of 
commands.

-- Darin

___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev


Re: [webkit-dev] run-bindings-tests

2011-09-08 Thread Adam Barth
2011/9/8 Darin Adler da...@apple.com:
 I find the bindings tests quite helpful. Because the perl script is so hard
 to read, it’s the changes in bindings script test results that I look at
 when reviewing changes to the bindings scripts. The fact that the results
 are checked in helps me review patches.
 It would be better to plug them in to the testing machinery in a better way.
 I don’t think contributors should have to learn how to run different types
 of commands.

Maybe we should teach run-webkit-tests how to run all the various
testing scripts?  That way there's just one command to learn and run.
run-bindings-tests takes 3.6s on my machine, so I suspect the extra
time involved isn't noticeable.

Adam
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev


Re: [webkit-dev] run-bindings-tests

2011-09-08 Thread Eric Seidel
If the objection against run-bindings-tests is that they're not part
of some larger test script which developers can run locally, it's very
easy to add a wrapper script which runs all known testing harnesses.

The test tests which currently run on the bots include:
run-webkit-tests (minutes)
run-javascriptcore-tests (45s)
test-webkitpy (32s)
test-webkitperl (2.0s)
run-binding-tests (2.4s)
run-api-tests (failed on my machine, so I can't tell you how long)

There are a couple other scripts which run on specific platforms, but
that's the core set.

run-webkit-tests is the bulk of the time, taking multiple minutes on a
modern machine (even with NRWT).

I'm happy to write a run-all-tests script which runs all known tests
that platform can handle. :)

-eric


On Thu, Sep 8, 2011 at 12:19 PM, Oliver Hunt oli...@apple.com wrote:

 On Sep 8, 2011, at 11:55 AM, James Robinson wrote:

 We used to not run these tests on the bots.  This meant that people would
 change the bindings code and not update the expected results, so the
 expected results were always massively out of date.  This meant when
 patching the bindings scripts you could not rely on run-bindings-tests at
 all, because the expectations were already broken before you made any
 changes!  This it not theoretical, it happened to be multiple times and I
 know I'm not the only one.
 The real problem here is that people check in without looking at the bots
 and then do not respond when the bots go red.  That's a people problem.

 The real problem is that we have a test suite: run-webkit-tests, that
 everyone runs (it's even mentioned at step 4 on
 http://www.webkit.org/coding/contributing.html , which is apparently not
 running all the tests.
 I run-webkit-tests before i commit (new-run-webkit-tests has ensured that
 any prior complains about time taken no longer exist, so kudos to those folk
 \o/ ), and yet I end up breaking the build in a way that would show up
 locally if the tests were simply run.  run-webkit-tests should run all of
 the webkit tests -- not some subset, all of them.  If failing a cross
 platform test can turn the bots red, then that test should be covered by
 run-webkit-tests.
 The other problem is people check in without looking at the bots.  I do
 try to watch the bots, but the time between me landing and the bots actually
 going red can literally be hours.  Of course i'm away from irc/email
 whatever when I land a patch at 4pm and it turns the bots red at 11pm.
 I appreciate this isn't as much of a problem for people who don't work on
 code for which all changes heralds a world rebuild and effect (apparently)
 every single test that exists in the repository, but it's certainly
 frustrating for those of us who do.
 (This is ignoring the overly aggressive rollouts of large patches the break
 only single platforms due to platform specific code that is difficult for
 anyone outside of that platform to fix)
 --Oliver

 - James


 - WBR, Alexey Proskuryakov

 ___
 webkit-dev mailing list
 webkit-dev@lists.webkit.org
 http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev

 ___
 webkit-dev mailing list
 webkit-dev@lists.webkit.org
 http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev


 ___
 webkit-dev mailing list
 webkit-dev@lists.webkit.org
 http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev


___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev


Re: [webkit-dev] run-bindings-tests

2011-09-08 Thread Ryosuke Niwa
On Thu, Sep 8, 2011 at 12:19 PM, Oliver Hunt oli...@apple.com wrote:

 The other problem is people check in without looking at the bots.  I do
 try to watch the bots, but the time between me landing and the bots actually
 going red can literally be hours.  Of course i'm away from irc/email
 whatever when I land a patch at 4pm and it turns the bots red at 11pm.


I agree. Some bots take 3-4 hours to run (especially if you touch Node.h,
platform.h, etc... that cause full or near-full rebuild of WebCore), and
it's not realistic for us to expect contributors to keep watching at the
waterfall until all bots go green. Sure, they should be ultimately
responsible for keeping them green and should be checking them back but the
fact most of bots always have some tests failing make this process
needlessly harder as well.

On Thu, Sep 8, 2011 at 12:27 PM, Adam Barth aba...@webkit.org wrote:

 Maybe we should teach run-webkit-tests how to run all the various
 testing scripts?  That way there's just one command to learn and run.
 run-bindings-tests takes 3.6s on my machine, so I suspect the extra
 time involved isn't noticeable.


On Thu, Sep 8, 2011 at 12:29 PM, Eric Seidel esei...@google.com wrote:

 If the objection against run-bindings-tests is that they're not part
 of some larger test script which developers can run locally, it's very
 easy to add a wrapper script which runs all known testing harnesses.

 The test tests which currently run on the bots include:
 run-webkit-tests (minutes)
 run-javascriptcore-tests (45s)
 test-webkitpy (32s)
 test-webkitperl (2.0s)
 run-binding-tests (2.4s)
 run-api-tests (failed on my machine, so I can't tell you how long)

 There are a couple other scripts which run on specific platforms, but
 that's the core set.


I like this idea!  But I think having each kind of test as a separate step
on buildbot so this wrapper script is either used only locally or else we
need to teach buildslave/buildbot how to talk to this script.

- Ryosuke
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev


Re: [webkit-dev] run-bindings-tests

2011-09-08 Thread Darin Adler
On Sep 8, 2011, at 12:29 PM, Eric Seidel wrote:

 I'm happy to write a run-all-tests script which runs all known tests that 
 platform can handle. :)

I think run-webkit-tests should be this. We can come up with a new name for the 
“just run the tests in the LayoutTests directory” tool.

A bigger problem is the different way that all the various tests indicate tests 
have run, succeeded, or failed. If you try to run all of these it’s not trivial 
to figure out what happened.

-- Darin

___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev


Re: [webkit-dev] run-bindings-tests

2011-09-08 Thread Alexey Proskuryakov

08.09.2011, в 12:25, Darin Adler написал(а):

 I find the bindings tests quite helpful. Because the perl script is so hard 
 to read, it’s the changes in bindings script test results that I look at when 
 reviewing changes to the bindings scripts. The fact that the results are 
 checked in helps me review patches.

OK, then they are valuable indeed.

However, I still feel that there is a disconnect between the desired effect 
(provide a diff in a patch for review) and the implementation (tests that can 
pass or fail). This also puts the burden of maintaining the results on people 
who needn't care about them - for example, Oliver's patch clearly didn't need 
someone look over generated code changes.

I'm not sure what the better solution would be though. Perhaps a bot could 
provide a diff of DerivedSources for any patch that touches code generators, 
but I'm not volunteering to implement one :-)

- WBR, Alexey Proskuryakov

___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev