Skip to content

Commit

Permalink
fix timing issue breaking node --test serialization
Browse files Browse the repository at this point in the history
The TestMap class was getting confused between the `Test` object and the
`TestBase` object, becuase the `.t` member can't be accessed while the
Test class is still constructing.

The intent was for it to swap over to the t.t once available, but that
wasn't happening properly if multiple a subtests were created in the
same tick as the root TAP object being created.

In any event, it was overly clever. Just slap a pseudorandom key on it
with a known Symbol property, and use that as the key in the diags and
subs maps.
  • Loading branch information
isaacs committed Oct 2, 2023
1 parent 69c31a8 commit 5811079
Show file tree
Hide file tree
Showing 3 changed files with 66 additions and 59 deletions.
51 changes: 27 additions & 24 deletions src/node-serialize/src/test-map.ts
Original file line number Diff line number Diff line change
@@ -1,37 +1,40 @@
// A map of test objects, but make sure that we don't get confused
// if we get the Test proxy or the underlying TestBase.

import type { Base, TestBase } from '@tapjs/core'
import { Base } from '@tapjs/core'

const kSerializationKey = Symbol.for('@tapjs/node-serialize.key')
const getKey = (
t: Base & { [kSerializationKey]?: string }
): string => {
const k = t[kSerializationKey]
if (k) return k
const n = String(Math.random())
Object.defineProperty(t, kSerializationKey, {
value: n,
writable: false,
configurable: true,
enumerable: false,
})
return n
}

export class TestMap<T extends {}> extends Map<Base, T> {
get(t: Base): T | undefined {
const tt = (t as TestBase).t
if (tt !== undefined && tt !== t) {
let vt = super.get((t as TestBase).t)
if (vt === undefined) {
const v = super.get(t)
if (v !== undefined) {
super.set(tt, v)
super.delete(t)
return v
}
constructor(items?: [Base, T][]) {
super()
if (items) {
for (const [t, v] of items) {
this.set(t, v)
}
return vt
}
return super.get(t)
}
get(t: Base): T | undefined {
return super.get(getKey(t) as unknown as Base)
}
has(t: Base) {
const tt = (t as TestBase).t
return super.has(t) || super.has(tt)
return super.has(getKey(t) as unknown as Base)
}
set(t: Base, v: T) {
const tt = (t as TestBase).t
if (tt !== undefined && tt !== t) {
super.delete(t)
super.set(tt, v)
} else {
super.set(t, v)
}
return this
return super.set(getKey(t) as unknown as Base, v)
}
}
54 changes: 25 additions & 29 deletions src/node-serialize/test/on-add.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { at } from '@tapjs/stack'
import t, { Base, Minimal } from 'tap'
import {setTimeout} from 'timers/promises'
import { setTimeout } from 'timers/promises'
import { onAddFn } from '../src/on-add.js'
import { TestMap } from '../src/test-map.js'

Expand Down Expand Up @@ -46,45 +46,41 @@ t.test('track the things', async t => {
t.matchOnly(
{
diags,
subsMap: [...subsMap].map(([p, subs]) => [
p.name,
subs.map(s => s.name),
]),
diagsMap: [...diagsMap].map(([p, diags]) => [p.name, diags]),
subsMap: [...subsMap.values()].map(subs =>
subs.map(s => s.name)
),
diagsMap: [...diagsMap.values()],
},
{
diags: [
['comment via commentMethod', ['suite 1 comment']],
['comment via commentMethod', ['suite a comment']],
],
subsMap: [
['TAP', ['suite 1', 'suite a']],
['suite 1', ['test 1', 'test 2']],
['test 1', []],
['test 2', []],
['suite a', ['test b', 'test c']],
['test b', []],
['test c', []],
['suite 1', 'suite a'],
['test 1', 'test 2'],
[],
[],
['test b', 'test c'],
[],
[],
],
diagsMap: [
[
'TAP',
[
{
file: String,
line: Number,
column: Number,
nesting: 0,
message: 'hello { world: true }',
},
],
{
file: String,
line: Number,
column: Number,
nesting: 0,
message: 'hello { world: true }',
},
],
['suite 1', []],
['test 1', []],
['test 2', []],
['suite a', []],
['test b', []],
['test c', []],
[],
[],
[],
[],
[],
[],
],
}
)
Expand Down
20 changes: 14 additions & 6 deletions src/node-serialize/test/test-map.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
import t, { Test, TestBase } from 'tap'
import t, { Test } from 'tap'
import { TestMap } from '../src/test-map.js'

const tm = new TestMap<number>()
const testOne = { t: {} } as unknown as TestBase
const testTwo = {} as unknown as TestBase
const testOne = new Test({ name: 'one' })
const testTwo = new Test({ name: 'two' })
tm.set(testOne, 1)
t.equal(tm.has(testOne), true)
t.equal(tm.get(testOne), 1)
Expand All @@ -12,7 +12,15 @@ t.equal(tm.get(testOne.t), 1)
tm.set(testTwo, 2)
t.equal(tm.has(testTwo), true)
t.equal(tm.get(testTwo), 2)
testTwo.t = {} as unknown as Test
t.equal(tm.has(testTwo), true)
t.equal(tm.get(testTwo), 2)
t.equal(tm.has(testTwo.t), true)
t.equal(tm.get(testTwo.t), 2)

const tm2 = new TestMap<number>([[testOne, 1], [testTwo, 2]])
t.equal(tm2.has(testOne), true)
t.equal(tm2.get(testOne), 1)
t.equal(tm2.has(testOne.t), true)
t.equal(tm2.get(testOne.t), 1)
t.equal(tm2.has(testTwo), true)
t.equal(tm2.get(testTwo), 2)
t.equal(tm2.has(testTwo.t), true)
t.equal(tm2.get(testTwo.t), 2)

0 comments on commit 5811079

Please sign in to comment.