Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
9325007:
Fix frogium build by disabling test for frog, frogsh AND frogium. (Closed)
|
Created:
8 years, 10 months ago by srdjan Modified:
8 years, 10 months ago CC:
reviews_dartlang.org, vm-dev_dartlang.org Visibility:
Public. |
DescriptionFix frogium build by disabling test for frog, frogsh AND frogium.
Committed: https://code.google.com/p/dart/source/detail?r=3874
Patch Set 1 #
Total comments: 6
Messages
Total messages: 7 (0 generated)
TBR
dbc https://chromiumcodereview.appspot.com/9325007/diff/1/tests/language/language... File tests/language/language.status (right): https://chromiumcodereview.appspot.com/9325007/diff/1/tests/language/language... tests/language/language.status:421: IntrinsifiedMethodsTest: Fail # Issue 1470 I suspect this belongs up in the big block above: $component == frogsh || $component == frog || ($component == webdriver && ($browser == chrome || $browser == ff || $browser == safari || $browser == ie)) || $component == frogium (unrelated note: I wonder if we can refactor "$component == webdriver && ($browser == chrome || $browser == ff || $browser == safari || $browser == ie)" into just "$component == webdriver"
LGTM -Ivan https://chromiumcodereview.appspot.com/9325007/diff/1/tests/language/language... File tests/language/language.status (right): https://chromiumcodereview.appspot.com/9325007/diff/1/tests/language/language... tests/language/language.status:421: IntrinsifiedMethodsTest: Fail # Issue 1470 On 2012/02/02 21:17:01, John Messerly wrote: > I suspect this belongs up in the big block above: > > $component == frogsh || $component == frog || ($component == webdriver && > ($browser == chrome || $browser == ff || $browser == safari || $browser == ie)) > || $component == frogium > > (unrelated note: I wonder if we can refactor "$component == webdriver && > ($browser == chrome || $browser == ff || $browser == safari || $browser == ie)" > into just "$component == webdriver" It is nearly impossible for any of us to know what tests run under what circumstance on what browser on which OS and what flags. So if you strongly feel this should go somewhere else, please feel free to shuffle it around.
https://chromiumcodereview.appspot.com/9325007/diff/1/tests/language/language... File tests/language/language.status (right): https://chromiumcodereview.appspot.com/9325007/diff/1/tests/language/language... tests/language/language.status:421: IntrinsifiedMethodsTest: Fail # Issue 1470 On 2012/02/02 21:17:01, John Messerly wrote: > I suspect this belongs up in the big block above: > > $component == frogsh || $component == frog || ($component == webdriver && > ($browser == chrome || $browser == ff || $browser == safari || $browser == ie)) > || $component == frogium > > (unrelated note: I wonder if we can refactor "$component == webdriver && > ($browser == chrome || $browser == ff || $browser == safari || $browser == ie)" > into just "$component == webdriver" +1 to jmesserly's first comment. There is talk of also adding Opera, so I wouldn't remove all the browser list just yet (sorry so verbose).
https://chromiumcodereview.appspot.com/9325007/diff/1/tests/language/language... File tests/language/language.status (right): https://chromiumcodereview.appspot.com/9325007/diff/1/tests/language/language... tests/language/language.status:421: IntrinsifiedMethodsTest: Fail # Issue 1470 On 2012/02/02 21:39:52, Ivan Posva wrote: > On 2012/02/02 21:17:01, John Messerly wrote: > > I suspect this belongs up in the big block above: > > > > $component == frogsh || $component == frog || ($component == webdriver && > > ($browser == chrome || $browser == ff || $browser == safari || $browser == > ie)) > > || $component == frogium > > > > (unrelated note: I wonder if we can refactor "$component == webdriver && > > ($browser == chrome || $browser == ff || $browser == safari || $browser == > ie)" > > into just "$component == webdriver" > > It is nearly impossible for any of us to know what tests run under what > circumstance on what browser on which OS and what flags. So if you strongly feel > this should go somewhere else, please feel free to shuffle it around. Totally understand! I get lost in all of our configurations for Dart too :-) What caught my eye here is that this change added a new set of permutations (frog || frogsh || frogium). That's why I was suggesting we move this test into the other block, which is intended to mean "this test fails in frog, regardless of configuration". Also, it means one less permutation to worry about :)
https://chromiumcodereview.appspot.com/9325007/diff/1/tests/language/language... File tests/language/language.status (right): https://chromiumcodereview.appspot.com/9325007/diff/1/tests/language/language... tests/language/language.status:421: IntrinsifiedMethodsTest: Fail # Issue 1470 On 2012/02/02 21:50:04, John Messerly wrote: > On 2012/02/02 21:39:52, Ivan Posva wrote: > > On 2012/02/02 21:17:01, John Messerly wrote: > > > I suspect this belongs up in the big block above: > > > > > > $component == frogsh || $component == frog || ($component == webdriver && > > > ($browser == chrome || $browser == ff || $browser == safari || $browser == > > ie)) > > > || $component == frogium > > > > > > (unrelated note: I wonder if we can refactor "$component == webdriver && > > > ($browser == chrome || $browser == ff || $browser == safari || $browser == > > ie)" > > > into just "$component == webdriver" > > > > It is nearly impossible for any of us to know what tests run under what > > circumstance on what browser on which OS and what flags. So if you strongly > feel > > this should go somewhere else, please feel free to shuffle it around. > > Totally understand! I get lost in all of our configurations for Dart too :-) > > What caught my eye here is that this change added a new set of permutations > (frog || frogsh || frogium). That's why I was suggesting we move this test into > the other block, which is intended to mean "this test fails in frog, regardless > of configuration". Also, it means one less permutation to worry about :) Regardless of OS/browser combination, there are *four* components being tested: frog, frogsh, frogium, and webdriver. If you just specify webdriver as failing for a particular test, you don't need to write out the browser combinations.
https://chromiumcodereview.appspot.com/9325007/diff/1/tests/language/language... File tests/language/language.status (right): https://chromiumcodereview.appspot.com/9325007/diff/1/tests/language/language... tests/language/language.status:421: IntrinsifiedMethodsTest: Fail # Issue 1470 On 2012/02/02 21:50:04, John Messerly wrote: > On 2012/02/02 21:39:52, Ivan Posva wrote: > > On 2012/02/02 21:17:01, John Messerly wrote: > > > I suspect this belongs up in the big block above: > > > > > > $component == frogsh || $component == frog || ($component == webdriver && > > > ($browser == chrome || $browser == ff || $browser == safari || $browser == > > ie)) > > > || $component == frogium > > > > > > (unrelated note: I wonder if we can refactor "$component == webdriver && > > > ($browser == chrome || $browser == ff || $browser == safari || $browser == > > ie)" > > > into just "$component == webdriver" > > > > It is nearly impossible for any of us to know what tests run under what > > circumstance on what browser on which OS and what flags. So if you strongly > feel > > this should go somewhere else, please feel free to shuffle it around. > > Totally understand! I get lost in all of our configurations for Dart too :-) > > What caught my eye here is that this change added a new set of permutations > (frog || frogsh || frogium). That's why I was suggesting we move this test into > the other block, which is intended to mean "this test fails in frog, regardless > of configuration". Also, it means one less permutation to worry about :) Could you please add comments to the file describing what the different permutations mean? Thanks! |
||||||||||||||||||||||||||||||||||||||||||||||
