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

test_runner: t.after should respect the first-in-last-out principle like Golang's defer #55853

Open
axetroy opened this issue Nov 14, 2024 · 1 comment
Labels
feature request Issues that request new features to be added to Node.js. test_runner Issues and PRs related to the test runner subsystem.

Comments

@axetroy
Copy link

axetroy commented Nov 14, 2024

Version

v22.10.0

Platform

No response

Subsystem

No response

What steps will reproduce the bug?

1. Create a test file

import fs from 'node:fs'
import path from 'node:path'
import test from 'node:test'

test('basic', async (t) => {
    const testDir = path.join(import.meta.dirname, 'logs')

    fs.mkdirSync(testDir, { recursive: true })

    t.after(() => {
        console.log('remove test dir')
        fs.rmdirSync(testDir, { recursive: true })
    })

    fs.writeFileSync(path.join(testDir, 'test.log'), 'hello world!')

    t.after(() => {
        console.log('remove test file')
        fs.unlinkSync(path.join(testDir, 'test.log'))
    })

    // do staff...
})

How often does it reproduce? Is there a required condition?

None

What is the expected behavior? Why is that the expected behavior?

t.after should follow the first-in, last-out principle

According to the above code, the file should be deleted first, then the directory

What do you see instead?

✖ basic (7.4021ms)
  Error: ENOENT: no such file or directory, unlink 'project\folder\logs\test.log'
      at Object.unlinkSync (node:fs:1871:11)
      at TestContext.<anonymous> (file:///path/to/test.test.mjs:19:12)
      at TestHook.runInAsyncScope (node:async_hooks:211:14)
      at TestHook.run (node:internal/test_runner/test:934:25)
      at TestHook.run (node:internal/test_runner/test:1225:18)
      at TestHook.run (node:internal/util:543:20)
      at node:internal/test_runner/test:853:20
      at async Test.runHook (node:internal/test_runner/test:851:7)
      at async after (node:internal/test_runner/test:893:9)
      at async Test.run (node:internal/test_runner/test:942:7) {

Additional information

the first-in-last-out principle is more reasonable and practical. It is useful in many scenarios.

I'm not sure why it was designed in the form of a queue. Is there anything special about it?

@RedYetiDev
Copy link
Member

RedYetiDev commented Nov 14, 2024

IMO this code snippet is functioning as intended. The hooks are running in the order they were received.

I don't think we should reverse that order, as IMO it'll only cause confusion and breakages.

-1 from me

@RedYetiDev RedYetiDev added feature request Issues that request new features to be added to Node.js. test_runner Issues and PRs related to the test runner subsystem. labels Nov 14, 2024
@github-project-automation github-project-automation bot moved this to Awaiting Triage in Node.js feature requests Nov 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request Issues that request new features to be added to Node.js. test_runner Issues and PRs related to the test runner subsystem.
Projects
Status: Awaiting Triage
Development

No branches or pull requests

2 participants