Improve VM Test Reporting (#4297)

* Add per-directory reporting to VM test runners (as custom vitest reporter)

* Integrate state reporting into blockchain test runner

Author: holgerd77

packages/vm/package.json

@@ -54,13 +54,13 @@
     "test:blockchain:transitionForks": "echo 'ByzantiumToConstantinopleFixAt5 EIP158ToByzantiumAt5 FrontierToHomesteadAt5 HomesteadToDaoAt5 HomesteadToEIP150At5 BerlinToLondonAt5' | xargs -n1 | xargs -I {} sh -c 'VITE_FORK={} VITE_VERIFY_TEST_AMOUNT_ALL_TESTS=1 npx vitest test/tester/blockchain.spec.ts'",
     "test:blockchain:buildIntegrity": "npm run test:blockchain -- --file='randomStatetest303'",
     "test:buildIntegrity": "npm run test:state -- --test='stackOverflow'",
-    "test:est:stable:state": "TEST_PATH=../execution-spec-tests/stable/state_tests npx vitest run test/tester/executionSpecState.test.ts",
-    "test:est:stable:blockchain": "TEST_PATH=../execution-spec-tests/stable/blockchain_tests npx vitest run test/tester/executionSpecBlockchain.test.ts",
-    "test:est:dev:state": "TEST_PATH=../execution-spec-tests/dev/state_tests npx vitest run test/tester/executionSpecState.test.ts",
+    "test:est:stable:state": "TEST_PATH=../execution-spec-tests/stable/state_tests npx vitest run --reporter=default --reporter=./test/tester/util/perDirectoryReporter.ts test/tester/executionSpecState.test.ts",
+    "test:est:stable:blockchain": "TEST_PATH=../execution-spec-tests/stable/blockchain_tests npx vitest run --reporter=default --reporter=./test/tester/util/perDirectoryReporter.ts test/tester/executionSpecBlockchain.test.ts",
+    "test:est:dev:state": "TEST_PATH=../execution-spec-tests/dev/state_tests npx vitest run --reporter=default --reporter=./test/tester/util/perDirectoryReporter.ts test/tester/executionSpecState.test.ts",
     "test:est:dev:blockchain": "npm run test:est:dev:blockchain:v700",
-    "test:est:dev:blockchain:v700": "TEST_PATH=../execution-spec-tests/dev/blockchain_tests/amsterdam/v700_mixed_with_other_eips npx vitest run test/tester/executionSpecBlockchain.test.ts",
-    "test:est:dev:blockchain:v301": "TEST_PATH=../execution-spec-tests/dev/blockchain_tests/amsterdam/v301_single_bal_no_bal_defs npx vitest run test/tester/executionSpecBlockchain.test.ts",
-    "test:est:dev:blockchain:v200": "TEST_PATH=../execution-spec-tests/dev/blockchain_tests/amsterdam/v200_bal_defs_somewhat_outdated npx vitest run test/tester/executionSpecBlockchain.test.ts",
+    "test:est:dev:blockchain:v700": "TEST_PATH=../execution-spec-tests/dev/blockchain_tests/amsterdam/v700_mixed_with_other_eips npx vitest run --reporter=default --reporter=./test/tester/util/perDirectoryReporter.ts test/tester/executionSpecBlockchain.test.ts",
+    "test:est:dev:blockchain:v301": "TEST_PATH=../execution-spec-tests/dev/blockchain_tests/amsterdam/v301_single_bal_no_bal_defs npx vitest run --reporter=default --reporter=./test/tester/util/perDirectoryReporter.ts test/tester/executionSpecBlockchain.test.ts",
+    "test:est:dev:blockchain:v200": "TEST_PATH=../execution-spec-tests/dev/blockchain_tests/amsterdam/v200_bal_defs_somewhat_outdated npx vitest run --reporter=default --reporter=./test/tester/util/perDirectoryReporter.ts test/tester/executionSpecBlockchain.test.ts",
     "test:state": "tsx ./test/tester/vitest-wrapper.ts",
     "test:state:allForks": "npm run test:state:newForks && npm run test:state:oldForks && npm run test:state:transitionForks",
     "test:state:newForks": "echo 'Prague' | xargs -n1 | xargs -I {} sh -c 'VITE_FORK={} VITE_VERIFY_TEST_AMOUNT_ALL_TESTS=1 npx vitest test/tester/state.spec.ts'",

packages/vm/test/tester/executionSpecBlockchain.test.ts

@@ -21,6 +21,7 @@ import { createVM, runBlock } from '../../src/index.ts'
 import { setupPreConditions } from '../util.ts'
 import { createCommonForFork, loadExecutionSpecFixtures } from './executionSpecTestLoader.ts'
 import { compareBAL } from './util/balComparatorAI.ts'
+import { annotateFixture } from './util/perDirectoryReporter.ts'
 
 const customFixturesPath = process.env.TEST_PATH ?? '../execution-spec-tests'
 const fixturesPath = path.resolve(customFixturesPath)
@@ -77,8 +78,9 @@ if (fs.existsSync(fixturesPath) === false) {
       return
     }
 
-    for (const { id, fork, data } of fixtures) {
-      it(`${fork}: ${id}`, async () => {
+    for (const { id, fork, filePath, data } of fixtures) {
+      it(`${fork}: ${id}`, async ({ task }) => {
+        annotateFixture(task, filePath, fixturesPath, 'blockchain tests')
         await runBlockchainTestCase(fork, data, assert, kzg)
       }, 360000) // 6 minutes
     }
@@ -121,124 +123,170 @@ export async function runBlockchainTestCase(
 
   let parentBlock = genesisBlock
 
-  for (const {
-    rlp,
-    expectException,
-    blockHeader,
-    rlp_decoded,
-    blockAccessList,
-  } of testData.blocks) {
-    const expectedHash = blockHeader?.hash ?? rlp_decoded?.blockHeader?.hash ?? undefined
-    let block: Block | undefined
-    try {
-      block = createBlockFromRLP(hexToBytes(rlp), { common: vm.common, setHardfork: true })
-      //t.equal(bytesToHex(block.serialize()), rlp, 'correct block RLP')
-      if (expectedHash !== undefined) {
-        //t.equal(bytesToHex(block.hash()), expectedHash, 'correct block hash')
-      }
-      const result = await runBlock(vm, {
-        block,
-        root: parentBlock.header.stateRoot,
-        setHardfork: true,
-      })
-      await vm.blockchain.putBlock(block)
-      parentBlock = block
-      t.notExists(expectException, `Should have thrown with: ${expectException}`)
+  // Capture errors from block processing so post-state checks still run; this
+  // surfaces state-divergence details even when a block throws unexpectedly.
+  let runError: Error | undefined
 
-      // Check if the block level access list is correct
-      if (common.isActivatedEIP(7928)) {
-        let balDiffMessage = ''
-        if (blockAccessList !== undefined) {
-          const expectedBAL = createBlockLevelAccessListFromJSON(blockAccessList)
-          // Use the BAL comparator to show a colored diff of any mismatches
-          // Pass false to skip console output during test, we'll include it in the assertion
-          const { diffString } = compareBAL(
-            expectedBAL.raw(),
-            result.blockLevelAccessList!.raw(),
-            false,
-          )
-          balDiffMessage = diffString
+  try {
+    for (const {
+      rlp,
+      expectException,
+      blockHeader,
+      rlp_decoded,
+      blockAccessList,
+    } of testData.blocks) {
+      const expectedHash = blockHeader?.hash ?? rlp_decoded?.blockHeader?.hash ?? undefined
+      let block: Block | undefined
+      try {
+        block = createBlockFromRLP(hexToBytes(rlp), { common: vm.common, setHardfork: true })
+        //t.equal(bytesToHex(block.serialize()), rlp, 'correct block RLP')
+        if (expectedHash !== undefined) {
+          //t.equal(bytesToHex(block.hash()), expectedHash, 'correct block hash')
+        }
+        const result = await runBlock(vm, {
+          block,
+          root: parentBlock.header.stateRoot,
+          setHardfork: true,
+        })
+        await vm.blockchain.putBlock(block)
+        parentBlock = block
+        t.notExists(expectException, `Should have thrown with: ${expectException}`)
+
+        // Check if the block level access list is correct
+        if (common.isActivatedEIP(7928)) {
+          let balDiffMessage = ''
+          if (blockAccessList !== undefined) {
+            const expectedBAL = createBlockLevelAccessListFromJSON(blockAccessList)
+            // Use the BAL comparator to show a colored diff of any mismatches
+            // Pass false to skip console output during test, we'll include it in the assertion
+            const { diffString } = compareBAL(
+              expectedBAL.raw(),
+              result.blockLevelAccessList!.raw(),
+              false,
+            )
+            balDiffMessage = diffString
+            t.deepEqual(
+              bytesToHex(expectedBAL.hash()),
+              bytesToHex(block.header.blockAccessListHash!),
+              `expected block level access list correct${balDiffMessage}`,
+            )
+          }
           t.deepEqual(
-            bytesToHex(expectedBAL.hash()),
+            bytesToHex(result.blockLevelAccessList!.hash()),
             bytesToHex(block.header.blockAccessListHash!),
-            `expected block level access list correct${balDiffMessage}`,
+            `generated block level access list correct${balDiffMessage}`,
           )
         }
-        t.deepEqual(
-          bytesToHex(result.blockLevelAccessList!.hash()),
-          bytesToHex(block.header.blockAccessListHash!),
-          `generated block level access list correct${balDiffMessage}`,
+      } catch (e: any) {
+        if (expectException === undefined) {
+          throw e
+        }
+        // Check if the block failed due to an expected exception
+        t.exists(
+          expectException,
+          `expectException should be defined.  Error: ${e.message}\n${e.stack}`,
         )
-      }
-    } catch (e: any) {
-      if (expectException === undefined) {
-        throw e
-      }
-      // Check if the block failed due to an expected exception
-      t.exists(
-        expectException,
-        `expectException should be defined.  Error: ${e.message}\n${e.stack}`,
-      )
 
-      if (expectException.includes('|') === true) {
-        const exceptions = expectException.split('|')
-        let i = 0
-        while (i < exceptions.length) {
-          try {
-            t.isTrue(
-              exceptions[i] in exceptionMessages,
-              `expectException: (${exceptions[i]}) should be in exceptionMessages.  Error: ${e.message}\n${e.stack}`,
-            )
-            t.match(
-              e.message,
-              exceptionMessages[exceptions[i]],
-              `Should have correct error for ${exceptions[i]}`,
-            )
-            break
-          } catch {
-            if (i === exceptions.length - 1) {
-              t.fail(
-                `Should have thrown one of the following exceptions: ${expectException}.  Threw: ${e.message}`,
+        if (expectException.includes('|') === true) {
+          const exceptions = expectException.split('|')
+          let i = 0
+          while (i < exceptions.length) {
+            try {
+              t.isTrue(
+                exceptions[i] in exceptionMessages,
+                `expectException: (${exceptions[i]}) should be in exceptionMessages.  Error: ${e.message}\n${e.stack}`,
+              )
+              t.match(
+                e.message,
+                exceptionMessages[exceptions[i]],
+                `Should have correct error for ${exceptions[i]}`,
               )
               break
+            } catch {
+              if (i === exceptions.length - 1) {
+                t.fail(
+                  `Should have thrown one of the following exceptions: ${expectException}.  Threw: ${e.message}`,
+                )
+                break
+              }
+              i++
             }
-            i++
           }
+        } else {
+          t.isTrue(
+            expectException in exceptionMessages,
+            `expectException: (${expectException}) should be in exceptionMessages.  Error: ${e.message}\n${e.stack}`,
+          )
+          // Check if the error message matches the expected exception
+          t.match(
+            e.message,
+            exceptionMessages[expectException],
+            `Should have correct error for ${expectException} -- got: ${e.message}`,
+          )
         }
-      } else {
-        t.isTrue(
-          expectException in exceptionMessages,
-          `expectException: (${expectException}) should be in exceptionMessages.  Error: ${e.message}\n${e.stack}`,
-        )
-        // Check if the error message matches the expected exception
-        t.match(
-          e.message,
-          exceptionMessages[expectException],
-          `Should have correct error for ${expectException} -- got: ${e.message}`,
-        )
       }
     }
+  } catch (e: any) {
+    runError = e
   }
 
-  // Check final state after all blocks are processed
-  const head = await blockchain.getCanonicalHeadBlock()
-  t.equal(bytesToHex(head.hash()), testData.lastblockhash, `head block hash matches lastblockhash`)
+  // Always check final state and post state, even if block processing threw.
+  // Individual diffs go into `postFailures` so we can show them alongside any
+  // block-processing error rather than masking them with an early throw.
+  const postFailures: string[] = []
 
-  // Check post state
-  for (const address of Object.keys(testData.postState)) {
-    const account = await vm.stateManager.getAccount(createAddressFromString(address))
-    t.exists(account, `account should be defined.  Got: ${address}`)
-    const accountInfo = testData.postState[address]
-    t.equal(account.balance, hexToBigInt(accountInfo.balance), 'correct balance')
-    t.equal(account.nonce, hexToBigInt(accountInfo.nonce), 'correct nonce')
-    t.deepEqual(account.codeHash, keccak_256(hexToBytes(accountInfo.code)), 'correct code')
+  try {
+    const head = await blockchain.getCanonicalHeadBlock()
+    t.equal(
+      bytesToHex(head.hash()),
+      testData.lastblockhash,
+      `head block hash matches lastblockhash`,
+    )
+  } catch (e: any) {
+    postFailures.push(`head block hash: ${e?.message ?? String(e)}`)
+  }
 
-    for (const [key, value] of Object.entries(accountInfo.storage)) {
-      const keyBytes = setLengthLeft(hexToBytes(key as `0x${string}`), 32)
-      const storage = await vm.stateManager.getStorage(createAddressFromString(address), keyBytes)
-      t.equal(bytesToHex(storage), value, 'correct storage')
+  const postState = testData.postState ?? {}
+  for (const address of Object.keys(postState)) {
+    try {
+      const account = await vm.stateManager.getAccount(createAddressFromString(address))
+      t.exists(account, `account should be defined.  Got: ${address}`)
+      const accountInfo = postState[address]
+      t.equal(account.balance, hexToBigInt(accountInfo.balance), `correct balance (${address})`)
+      t.equal(account.nonce, hexToBigInt(accountInfo.nonce), `correct nonce (${address})`)
+      t.deepEqual(
+        account.codeHash,
+        keccak_256(hexToBytes(accountInfo.code)),
+        `correct code (${address})`,
+      )
+
+      for (const [key, value] of Object.entries(accountInfo.storage)) {
+        const keyBytes = setLengthLeft(hexToBytes(key as `0x${string}`), 32)
+        const storage = await vm.stateManager.getStorage(createAddressFromString(address), keyBytes)
+        t.equal(bytesToHex(storage), value, `correct storage[${key}] (${address})`)
+      }
+    } catch (e: any) {
+      postFailures.push(e?.message ?? String(e))
     }
   }
+
+  if (runError === undefined && postFailures.length === 0) return
+
+  // Build a combined failure. If a block-run error was the root cause, keep
+  // its original stack so vitest's source-mapped frames still point there.
+  const sections: string[] = []
+  if (runError !== undefined) sections.push(runError.message)
+  if (postFailures.length > 0) {
+    const header =
+      postFailures.length === 1
+        ? `Post-run state issue:`
+        : `Post-run state issues (${postFailures.length}):`
+    sections.push([header, ...postFailures.map((m) => `  - ${m}`)].join('\n'))
+  }
+
+  const combined = new Error(sections.join('\n\n'))
+  if (runError?.stack !== undefined) combined.stack = runError.stack
+  throw combined
 }
 
 // EthJS error messages mapped to expected exception types

packages/vm/test/tester/executionSpecState.test.ts

@@ -11,6 +11,7 @@ import { createVM } from '../../src/constructors.ts'
 import { runTx } from '../../src/runTx.ts'
 import { makeBlockFromEnv, makeTx, setupPreConditions } from '../util.ts'
 import { loadExecutionSpecFixtures, parseTest } from './executionSpecTestLoader.ts'
+import { annotateFixture } from './util/perDirectoryReporter.ts'
 
 const customFixturesPath = process.env.TEST_PATH ?? '../execution-spec-tests'
 const fixturesPath = path.resolve(customFixturesPath)
@@ -52,8 +53,9 @@ if (fs.existsSync(fixturesPath) === false) {
       return
     }
 
-    for (const { id, fork, data } of fixtures) {
-      it(`${fork}: ${id}`, async () => {
+    for (const { id, fork, filePath, data } of fixtures) {
+      it(`${fork}: ${id}`, async ({ task }) => {
+        annotateFixture(task, filePath, fixturesPath, 'state tests')
         const testCase = parseTest(fork, data)
         try {
           await runStateTestCase(fork, testCase, assert, kzg)