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

Use node test runner #1845

Merged
merged 5 commits into from
Oct 28, 2024
Merged

Conversation

sirenkovladd
Copy link
Contributor

@sirenkovladd sirenkovladd commented Sep 20, 2024

Checklist

use node --test instead of jest
See: #1837 (comment)

  • I have ensured my pull request is not behind the main or master branch of the original repository.
  • I have rebased all commits where necessary so that reviewing this pull request can be done without having to merge it first.
  • I have written a commit message that passes commitlint linting.
  • I have ensured that my code changes pass linting tests.
  • I have ensured that my code changes pass unit tests.
  • I have described my pull request and the reasons for code changes along with context if necessary.

})
.on('error', done)
.end()
})
})

it('should catch stream error', done => {
// TODO
Copy link
Contributor

Choose a reason for hiding this comment

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

What's the TODO regarding?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

thank you, forgot to delete

const assert = require('assert')
const Koa = require('../..')

process.env.NODE_ENV = 'test'
Copy link
Contributor

@kevinpeno kevinpeno Oct 20, 2024

Choose a reason for hiding this comment

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

Why is NODE_ENV now required specifically for this test?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

changed, moved this env to params

@kevinpeno kevinpeno self-assigned this Oct 20, 2024
@kevinpeno
Copy link
Contributor

@sirenkovladd, thanks for all the updates! I want to check in with the more seasoned team about the removed test before moving forward. I appreciate your patience!

Copy link
Contributor

@kevinpeno kevinpeno left a comment

Choose a reason for hiding this comment

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

Thanks for all the work!

@kevinpeno kevinpeno merged commit ce6b3b6 into koajs:master Oct 28, 2024
4 checks passed
@jonathanong
Copy link
Member

it seems like the tests do not automatically close when I run nom test locally. any ideas why?

@jonathanong
Copy link
Member

hmmm nevermind, it only happened once

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants