Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add test.debug to integration tests #14133

Merged
merged 3 commits into from
Aug 8, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -7,3 +7,4 @@ playwright-report/
blob-report/
playwright/.cache/
target/
.debug
142 changes: 87 additions & 55 deletions integrations/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,10 @@ interface TestContext {
}
}
type TestCallback = (context: TestContext) => Promise<void> | void
interface TestFlags {
only?: boolean
debug?: boolean
}

type SpawnActor = { predicate: (message: string) => boolean; resolve: () => void }

Expand All @@ -51,70 +55,41 @@ export function test(
name: string,
config: TestConfig,
testCallback: TestCallback,
{ only = false } = {},
{ only = false, debug = false }: TestFlags = {},
) {
return (only ? defaultTest.only : defaultTest)(
name,
{ timeout: TEST_TIMEOUT },
async (options) => {
let root = await fs.mkdtemp(
// On Windows CI, tmpdir returns a path containing a weird RUNNER~1 folder
// that apparently causes the vite builds to not work.
path.join(
process.env.CI && platform() === 'win32' ? homedir() : tmpdir(),
'tailwind-integrations',
),
)

async function write(filename: string, content: string): Promise<void> {
let full = path.join(root, filename)

if (filename.endsWith('package.json')) {
content = await overwriteVersionsInPackageJson(content)
}

// Ensure that files written on Windows use \r\n line ending
if (platform() === 'win32') {
content = content.replace(/\n/g, '\r\n')
}

let dir = path.dirname(full)
await fs.mkdir(dir, { recursive: true })
await fs.writeFile(full, content)
}

for (let [filename, content] of Object.entries(config.fs)) {
await write(filename, content)
let rootDir = debug
? path.join(REPO_ROOT, '.debug')
: // On Windows CI, tmpdir returns a path containing a weird RUNNER~1
// folder that apparently causes the vite builds to not work.
process.env.CI && platform() === 'win32'
? homedir()
: tmpdir()
await fs.mkdir(rootDir, { recursive: true })

let root = await fs.mkdtemp(path.join(rootDir, 'tailwind-integrations'))

if (debug) {
console.log('Running test in debug mode. File system will be written to:')
console.log(root)
console.log()
}

try {
execSync('pnpm install', { cwd: root })
} catch (error: any) {
console.error(error.stdout.toString())
console.error(error.stderr.toString())
throw error
}

let disposables: (() => Promise<void>)[] = []

async function dispose() {
await Promise.all(disposables.map((dispose) => dispose()))
try {
await fs.rm(root, { recursive: true, maxRetries: 5, force: true })
} catch (err) {
if (!process.env.CI) {
throw err
}
}
}

options.onTestFinished(dispose)

let context = {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was moved up so that pnpm install can use the same exec function that we also export

root,
async exec(command: string, childProcessOptions: ChildProcessOptions = {}) {
let cwd = childProcessOptions.cwd ?? root
if (debug && cwd !== root) {
let relative = path.relative(root, cwd)
if (relative[0] !== '.') relative = `./${relative}`
console.log(`> cd ${relative}`)
}
if (debug) console.log(`> ${command}`)
return execSync(command, {
cwd: root,
cwd,
stdio: 'pipe',
...childProcessOptions,
}).toString()
Expand All @@ -127,8 +102,15 @@ export function test(
rejectDisposal = reject
})

let cwd = childProcessOptions.cwd ?? root
if (debug && cwd !== root) {
let relative = path.relative(root, cwd)
if (relative[0] !== '.') relative = `./${relative}`
console.log(`> cd ${relative}`)
}
if (debug) console.log(`>& ${command}`)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could do the same here as above, but this now results in the same command log, but just with a different prefix:
image

I would maybe just drop this? 🤔

Suggested change
if (debug) console.log(`>& ${command}`)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@RobinMalfait I’m confused, how did you get to the above log? Were you doing both exec() and spawn()? (and if that is the case, would you not expect both to log? 🤔)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh wait, right, I'm confused as well haha. Not sure how I got the output twice. Please ignore.

let child = spawn(command, {
cwd: root,
cwd,
shell: true,
env: {
...process.env,
Expand Down Expand Up @@ -177,12 +159,14 @@ export function test(

child.stdout.on('data', (result) => {
let content = result.toString()
if (debug) console.log(content)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this have like a [stdout] prefix or something? Just so it's more easily distinguishable in the test output?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thinking out loud: should probably prefix every line of the output

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, maybe? It could help with the onStderr helper we have but I wonder why we need to be so diligent about this in the first place. In normal terminal apps you don't see the difference between stdout and stderr in the console either unless you pipe line stream to another location. I decided to replicate this here by using console.log for stdout and console.error for stderr

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I personally don't think the [stdout] or [stderr] prefix helps here because Vitest already shows that for us:

image

combined.push(['stdout', content])
stdoutMessages.push(content)
notifyNext(stdoutActors, stdoutMessages)
})
child.stderr.on('data', (result) => {
let content = result.toString()
if (debug) console.error(content)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ditto but with [stderr]

combined.push(['stderr', content])
stderrMessages.push(content)
notifyNext(stderrActors, stderrMessages)
Expand All @@ -195,6 +179,9 @@ export function test(
})

options.onTestFailed(() => {
// In debug mode, messages are logged to the console immediatly
if (debug) return

for (let [type, message] of combined) {
if (type === 'stdout') {
console.log(message)
Expand Down Expand Up @@ -253,7 +240,22 @@ export function test(
})
},
fs: {
write,
async write(filename: string, content: string): Promise<void> {
let full = path.join(root, filename)

if (filename.endsWith('package.json')) {
content = await overwriteVersionsInPackageJson(content)
}

// Ensure that files written on Windows use \r\n line ending
if (platform() === 'win32') {
content = content.replace(/\n/g, '\r\n')
}

let dir = path.dirname(full)
await fs.mkdir(dir, { recursive: true })
await fs.writeFile(full, content)
},
read(filePath: string) {
return fs.readFile(path.resolve(root, filePath), 'utf8')
},
Expand All @@ -277,13 +279,43 @@ export function test(
},
} satisfies TestContext

for (let [filename, content] of Object.entries(config.fs)) {
await context.fs.write(filename, content)
}

try {
context.exec('pnpm install')
} catch (error: any) {
console.error(error)
throw error
}

let disposables: (() => Promise<void>)[] = []

async function dispose() {
await Promise.all(disposables.map((dispose) => dispose()))
try {
if (debug) return
await fs.rm(root, { recursive: true, maxRetries: 5, force: true })
} catch (err) {
if (!process.env.CI) {
throw err
}
}
}

options.onTestFinished(dispose)

await testCallback(context)
},
)
}
test.only = (name: string, config: TestConfig, testCallback: TestCallback) => {
return test(name, config, testCallback, { only: true })
}
test.debug = (name: string, config: TestConfig, testCallback: TestCallback) => {
return test(name, config, testCallback, { only: true, debug: true })
}

// Maps package names to their tarball filenames. See scripts/pack-packages.ts
// for more details.
Expand Down
Loading