[BUG] TestSuite fails with Tap 18, even though all asserts pass or skip #964
Comments
Chaz0r
changed the title
|
I had difficulties with this too, but found a mysterious workaround (I work in a ts/fastify environment). It goes something like this |
|
@tengla That |
|
@Chaz0r so, couple of things here: You probably do not want to install But also, you can avoid even having to do that, by just letting TS figure out the argument type from the function type. https://github.com/tapjs/gh-964/blob/main/test/service-test-suite.ts import t from 'tap'
export default abstract class ServiceTestSuite {
test = t.test.bind(t)
constructor () {
this.defineTests()
}
abstract defineTests(): void
}At some point, you have to actually instantiate that class and call the Can you share the full code that's in play here, and the output of these commands? |
my bad, what made the non zero exit was actually lacking test coverage. |
If I dont do that, tap directly fails with the following error: I dont have an import in my service directly, so it looks like the import is missing somewhere implicitly. After I install the package manually, the issue is "fixed".
I will check that!
Yep, directly under the class definition. Pretty similar to this: export default class MyTest extends ServiceTestSuite {
//here all the test code
}
const myInstance = new MyTest()
myInstance.run()
Output of these commands are the following: tap config list tap list tap -Rtap Regarding the full code: //abstract application is the main app that starts our whole application. doing tests "inside" our application makes way more easy and worked fine with tap 16 and it looks like this still works pretty well
import t from "tap"
export abstract class ServiceTestSuite extends AbstractApplication {
async startApp(): Promise<void> {
//start the service and execute tests
}
async registerDependencies() {
await super.registerDependencies()
this.registerBaseMockDependencies()
}
private async registerBaseMockDependencies() {
//register mocks here to the DI
}
abstract configureBaseMockDependencies(): NameAndRegistrationPair<Cradle>[]
private async makeTestsuiteReadyWithOrm() {
await this.fastify.orm.initialize().then(async () => {
await this.makeTestSuiteReady()
}
).catch((error) => console.log(error))
}
private async makeTestSuiteReady() {
await this.fastify.ready()
await this.registerBefore()
await this.registerGlobalBeforeFunctions()
await this.registerAfter()
await this.registerGlobalAfterFunctions()
await this.registerJobs()
await this.defineTests()
}
private async registerGlobalBeforeFunctions() {
// eslint-disable-next-line @typescript-eslint/no-empty-function
await t.beforeEach(async () => {
//currently empty, wont be in the future
})
// eslint-disable-next-line @typescript-eslint/no-empty-function
await t.before(async () => {
//currently empty, wont be in the future
})
}
private async registerGlobalAfterFunctions() {
await t.afterEach(async () => {
//we refresh everything here so every test has a clean DI and clean DB
await this.refreshFastify()
})
await t.teardown(async () => {
this.fastify.close()
})
}
abstract defineTests(): void
async changeDependencies(nameAndRegistrationPairs: NameAndRegistrationPair<Cradle>[]) {
//here we can change dependencies for mocked dependencies
}
async registerJobs() {
const jobRegistrator = new JobRegistrator(this.fastify)
jobRegistrator.registerJobs()
}
async refreshFastify() {
//this resets all dependencies
}
abstract registerBefore(): void
abstract registerAfter(): void
getLogStreams(): SpeedifyLogStream[] | null {
return null
}
}
import t from "tap"
import { Test } from '@tapjs/test'
export default class FilterTest extends ServiceTestSuite {
defineTests(): void {
t.test('test filter without sorter exception', async (test: Test) => {
test.pass('this passes')
})
}
registerBefore(): void {
}
registerAfter(): void {
}
}
const testInstance = new FilterTest()
testInstance.startApp()What I figured out: If I remove the tap import and the before- and after functions and only use the imported tap inside the Test class, the issue seems to be gone. Edit: Beside that I still need the @tapjs/test package, otherwise I get the error I mentioned above. |
|
The error when you don't have Or, if you prefer, you can |
Nope, I removed all of those imports. I still get the error. Also another problem that drives me crazy with version 18 (16 did not have that problem) is the following: Like you suggested, I saved the test variable like this: The beforeEach and afterEach functions can only be used on t and not on t.test.
Technically in terms of tests, everything works now. All tests are running, all assertions green, nothing failing. But if I start my service now, the service stops because tap is running into a timeout. The service itself does not have any references to tap. I commented all the tests so the service even does not have any tests implemented at the moment. And only having the TestSuite in my library and the library including into my service results in the following error: TAP version 14
|
That's not possible. You might be getting a different error about
So, if I understand this correctly, you're importing tap, and calling some before/after registration methods, in your actual production application? If so, that's.... unusual. I've never even thought about doing that. It's shutting down after 30 seconds because the root test is registering itself on the process. Then it times out after the default test timeout of 30 seconds, because tests are intended to complete in less time than that. The expectation is that you wouldn't I feel like I really would be of more help to you here if I could get access to see some actual code failing locally. If it's too hard to carve it down a minimal reproduction, and you don't want to share a link publicly, feel free to DM me on the tapjs matrix chat or any discords/slacks we might both be in. I'm also happy to sign an MNDA if it's a private company thing. |
|
Thanks for the reply. Regarding the other issue: We use a Custom library for our services. This library contains plenty of code a service can use. Inside that Library there exists the TestSuite class. This class is NOT used in the production code. It is only used in the tests. The production code does not use test logic, test classes, tap or whatever is used for testing. It only exists in the lib so we can use it in tests. If we wouldnt have tests in the service, this TestSuite class would only float around in the library. Regarding code....yep....this is not that easy. It just is not a simple company topic, it is more a huge credit institute topic, which makes sharing code pretty hard >.< |
|
Did you figure it out? |
|
@Chaz0r I've found this as a result of some odd behaviour in my IDE, can you run the tests in an earlier version of node and see if they run without the error? I have a suite testing Fastify also, though way more rudimental than yours. Webstorm was hanging the suite while all tests passed, but running the tests from the terminal completed successfully. Then I checked node version and the terminal was running Node version 18.17.0. When I changed the terminal version to 20.8.0 it hung like it does in Webstorms run config (which is also using 20.8.0). |
|
Unsure if related but all codebases I've updated to v18 have started returning error code 1 even though the tests all look like they pass. I've tried disabling code coverage in case it had something to do with this but no dice. |
You need to put all these three flags in order to fully disable code coverage check: |
|
Technically you only need |
|
@digitalsadhu please post a new issue, with the info that the bug report asks for. "Looks like they all pass" is not sufficient detail to help you in any way. |
Is there an existing issue for this?
Have you read the
CONTRIBUTINGguide on posting bugs, andCODE_OF_CONDUCT?This issue exists in the latest tap version
Description
Hi folks,
I started to upgrade to the latest Tap 18 Version from the latest Tap 16 version. My backend service is a Typescript service with fastify in its core.
We are structuring our TestSuites like this:
It is important for us to define the tests in that function because we are doing plenty of things before the tests should be executed. We are not using the before and after methods of tap here because the things we do in the ServiceTestSuite are things which should happen even before the before method.
A working test looked like this with tap 16:
this.test has been defined in the TestSuite like this:
This worked pretty fine in Tap 16.
Now I started to migrate to 18.
I changed the tap version to the latest 18 and installed the @tapjs/test package.
After that I looked into the docs and figured out, that the imports have changed to
What I then did was to use those imports in the Test Class like this:
import t from "tap"
import { Test } from '@tapjs/test'
And with that I have some kind of Schrödingers test visibility.
This fails like it should:
It correctly says which assertion failed and why.
This does not show any assertion error (which is fine), but in the end, the whole TestSuite failes because now tests were found:
I removed all other TestSuites for the moment so this one is the only one that gets executed.
Also it looks like it works when I create a completely new TestSuite, where the tests are not defined inside a function. At least I don' get the issue there.
Reproduction
At the moment no idea :(
Environment
pnpm latest version
tap latest version (only run via pnpm)
node 20.6.1
The text was updated successfully, but these errors were encountered: