Annotate large string tests with largeHeap
Created attachment 298859 [details] Patch
Comment on attachment 298859 [details] Patch Seems fine to me, but I am too ignorant about our testing system to be the reviewer. What problem does this fix?
(In reply to comment #2) > Comment on attachment 298859 [details] > Patch > > Seems fine to me, but I am too ignorant about our testing system to be the > reviewer. What problem does this fix? The test will be skipped if you --memory-limited. The "//@" lines trigger the JSC test harness to eval that ruby statement in the context of run-jsc-stress-tests. I hadn't been aware of the largeHeap function but here it is inside run-jsc-stress-tests: def largeHeap if $memoryLimited $didAddRunCommand = true puts "Skipping #{$collectionName}/#{$benchmark}" end end The only thing I would write differently here is that: $didAddRunCommand = true puts "Skipping #{$collectionName}/#{$benchmark}" Is just the body of skip, so this should probably have been: def largeHeap skip if $memoryLimited end But if you do that, then it's not clear if it's better for tests to say: //@ largeHeap or for them to say: //@ skip if $memoryLimited I actually somewhat prefer the latter, since it answers Darin's question immediately. "largeHeap" does not tell me anything about the behavior while "skip if $memoryLimited" is self-explanatory.
(In reply to comment #3) > (In reply to comment #2) > > Comment on attachment 298859 [details] > > Patch > > > > Seems fine to me, but I am too ignorant about our testing system to be the > > reviewer. What problem does this fix? > > The test will be skipped if you --memory-limited. > > The "//@" lines trigger the JSC test harness to eval that ruby statement in > the context of run-jsc-stress-tests. I hadn't been aware of the largeHeap > function but here it is inside run-jsc-stress-tests: > > def largeHeap > if $memoryLimited > $didAddRunCommand = true > puts "Skipping #{$collectionName}/#{$benchmark}" > end > end > > The only thing I would write differently here is that: > > $didAddRunCommand = true > puts "Skipping #{$collectionName}/#{$benchmark}" > > Is just the body of skip, so this should probably have been: > > def largeHeap > skip if $memoryLimited > end > > But if you do that, then it's not clear if it's better for tests to say: > > //@ largeHeap > > or for them to say: > > //@ skip if $memoryLimited > > I actually somewhat prefer the latter, since it answers Darin's question > immediately. "largeHeap" does not tell me anything about the behavior while > "skip if $memoryLimited" is self-explanatory. Sounds fine. So, change `// @ largeHeap` use in JSTest / LayoutTests :)
Created attachment 298890 [details] Patch for landing
Committed r210775: <http://trac.webkit.org/changeset/210775>