lgtm On Mon, Sep 8, 2008 at 11:13 AM, <[EMAIL PROTECTED]> wrote: > Christian, > > I'd like you to do a code review. To review this change, run > > gvn review --project https://v8.googlecode.com/svn [EMAIL PROTECTED]/[EMAIL > PROTECTED] > > Alternatively, to review the latest snapshot of this change > branch, run > > gvn --project https://v8.googlecode.com/svn review [EMAIL > PROTECTED]/fix-no-snapshot-tests > > to review the following change: > > [EMAIL PROTECTED]/[EMAIL PROTECTED] | [EMAIL PROTECTED] | 2008-09-08 10:12:37 > +-100 (Mon, 08 Sep 2008) > > Description: > > Fix issues with running some of our tests with an embedded > snapshot. Changed the debug-script.js test to ignore the > exact number of extension scripts. > > > > Affected Paths: > M //branches/bleeding_edge/src/snapshot.h > M //branches/bleeding_edge/test/cctest/test-api.cc > M //branches/bleeding_edge/test/cctest/test-mark-compact.cc > M //branches/bleeding_edge/test/cctest/test-serialize.cc > M //branches/bleeding_edge/test/mjsunit/debug-script.js > > > This is a semiautomated message from "gvn mail". See > <http://code.google.com/p/gvn/> to learn more. > > Index: src/snapshot.h > =================================================================== > --- src/snapshot.h (^/branches/bleeding_edge/src/[EMAIL PROTECTED]) > +++ src/snapshot.h (^/changes/[EMAIL > PROTECTED]/fix-no-snapshot-tests/bleeding_edge/src/[EMAIL PROTECTED]) > @@ -37,8 +37,8 @@ class Snapshot { > // could be found. > static bool Initialize(const char* snapshot_file = NULL); > > - // Disable the use of the internal snapshot. > - static void DisableInternal() { size_ = 0; } > + // Returns whether or not the snapshot is enabled. > + static bool IsEnabled() { return size_ != 0; } > > // Write snapshot to the given file. Returns true if snapshot was written > // successfully. > Index: test/cctest/test-api.cc > =================================================================== > --- test/cctest/test-api.cc (^/branches/bleeding_edge/test/cctest/[EMAIL > PROTECTED]) > +++ test/cctest/test-api.cc (^/changes/[EMAIL > PROTECTED]/fix-no-snapshot-tests/bleeding_edge/test/cctest/[EMAIL PROTECTED]) > @@ -1429,7 +1429,7 @@ static const char* js_code_causing_out_of_memory = > // that come after them so they cannot run in parallel. > DISABLED_TEST(OutOfMemory) { > // It's not possible to read a snapshot into a heap with different > dimensions. > - v8::internal::Snapshot::DisableInternal(); > + if (v8::internal::Snapshot::IsEnabled()) return; > // Set heap limits. > static const int K = 1024; > v8::ResourceConstraints constraints; > @@ -1470,7 +1470,7 @@ v8::Handle<Value> ProvokeOutOfMemory(const v8::Arg > > DISABLED_TEST(OutOfMemoryNested) { > // It's not possible to read a snapshot into a heap with different > dimensions. > - v8::internal::Snapshot::DisableInternal(); > + if (v8::internal::Snapshot::IsEnabled()) return; > // Set heap limits. > static const int K = 1024; > v8::ResourceConstraints constraints; > @@ -1499,7 +1499,7 @@ DISABLED_TEST(OutOfMemoryNested) { > > TEST(HugeConsStringOutOfMemory) { > // It's not possible to read a snapshot into a heap with different > dimensions. > - v8::internal::Snapshot::DisableInternal(); > + if (v8::internal::Snapshot::IsEnabled()) return; > v8::HandleScope scope; > LocalContext context; > // Set heap limits. > Index: test/cctest/test-mark-compact.cc > =================================================================== > --- test/cctest/test-mark-compact.cc > (^/branches/bleeding_edge/test/cctest/[EMAIL PROTECTED]) > +++ test/cctest/test-mark-compact.cc (^/changes/[EMAIL > PROTECTED]/fix-no-snapshot-tests/bleeding_edge/test/cctest/[EMAIL PROTECTED]) > @@ -78,12 +78,12 @@ TEST(MarkingStack) { > TEST(Promotion) { > // Test the situation that some objects in new space are promoted to the > // old space > + if (Snapshot::IsEnabled()) return; > > // Ensure that we get a compacting collection so that objects are promoted > // from new space. > FLAG_gc_global = true; > FLAG_always_compact = true; > - Snapshot::DisableInternal(); > Heap::ConfigureHeap(2*256*KB, 4*MB); > > InitializeVM(); > @@ -110,7 +110,7 @@ TEST(Promotion) { > > > TEST(NoPromotion) { > - Snapshot::DisableInternal(); > + if (Snapshot::IsEnabled()) return; > Heap::ConfigureHeap(2*256*KB, 4*MB); > > // Test the situation that some objects in new space are promoted to > Index: test/cctest/test-serialize.cc > =================================================================== > --- test/cctest/test-serialize.cc > (^/branches/bleeding_edge/test/cctest/[EMAIL PROTECTED]) > +++ test/cctest/test-serialize.cc (^/changes/[EMAIL > PROTECTED]/fix-no-snapshot-tests/bleeding_edge/test/cctest/[EMAIL PROTECTED]) > @@ -191,14 +191,14 @@ TEST(SerializeInternal) { > // bootstrapped heap. > // (Smoke test.) > TEST(Serialize) { > - Snapshot::DisableInternal(); > + if (Snapshot::IsEnabled()) return; > Serialize(); > } > > > // Test that the heap isn't destroyed after a serialization. > TEST(SerializeNondestructive) { > - Snapshot::DisableInternal(); > + if (Snapshot::IsEnabled()) return; > StatsTable::SetCounterFunction(counter_function); > v8::HandleScope scope; > v8::Persistent<v8::Context> env = v8::Context::New(); > Index: test/mjsunit/debug-script.js > =================================================================== > --- test/mjsunit/debug-script.js > (^/branches/bleeding_edge/test/mjsunit/[EMAIL PROTECTED]) > +++ test/mjsunit/debug-script.js (^/changes/[EMAIL > PROTECTED]/fix-no-snapshot-tests/bleeding_edge/test/mjsunit/[EMAIL PROTECTED]) > @@ -53,9 +53,8 @@ for (i = 0; i < scripts.length; i++) { > } > } > > -// This has to be updated if the number of native and extension scripts > change. > +// This has to be updated if the number of native scripts change. > assertEquals(12, native_count); > -assertEquals(1, extension_count); > assertEquals(2, normal_count); // This script and mjsunit.js. > > // Test a builtins script. > @@ -75,8 +74,10 @@ assertEquals(Debug.ScriptType.Native, debug_delay_ > > // Test an extension script. > var extension_gc_script = Debug.findScript('v8/gc'); > -assertEquals('v8/gc', extension_gc_script.name); > -assertEquals(Debug.ScriptType.Extension, extension_gc_script.type); > +if (extension_gc_script) { > + assertEquals('v8/gc', extension_gc_script.name); > + assertEquals(Debug.ScriptType.Extension, extension_gc_script.type); > +} > > // Test a normal script. > var mjsunit_js_script = Debug.findScript(/mjsunit.js/); > >
--~--~---------~--~----~------------~-------~--~----~ v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev -~----------~----~----~----~------~----~------~--~---
