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

Implement .because() modifier for irregular tests #3227

Open
novemberborn opened this issue Jul 30, 2023 · 14 comments
Open

Implement .because() modifier for irregular tests #3227

novemberborn opened this issue Jul 30, 2023 · 14 comments

Comments

@novemberborn
Copy link
Member

novemberborn commented Jul 30, 2023

Based on discussion with @tommy-mitchell in #3224:

test.failing('does foo', t => {
  // ..
}).because('waiting on some/repo#123')

Should output:

✔ [expected fail] does foo
  - waiting on some/repo#123

This requires changes to https://github.com/avajs/ava/blob/main/lib/create-chain.js so that a chainable object is returned for test.failing(), test.skip() etc. Here we then need to add a because function. The reason should make its way to the reporter where it can be printed beneath the test title.

This should be reflected in the type definitions and documentation. Once we've made some progress on this, work needs to be done with https://github.com/avajs/eslint-plugin-ava to support this new modifier.

It should be a test failure if because() is called with anything but a non-empty string.

@sindresorhus

This comment was marked as resolved.

@novemberborn

This comment was marked as resolved.

@tommy-mitchell
Copy link
Contributor

This could have an ESLint rule as well, requiring a .because() description

@tommy-mitchell

This comment was marked as resolved.

@novemberborn

This comment was marked as resolved.

@adiSuper94
Copy link
Contributor

@novemberborn If no one has started working on the implementation, I'd like to take a crack at this.

@adiSuper94
Copy link
Contributor

Turned out to more complex than I anticipated. But I am still working on this.

@adiSuper94
Copy link
Contributor

adiSuper94 commented Sep 30, 2023

@novemberborn I have made some test and trying to test it. But looks like when I run npm run test it calls the old version of create-chain.js instead of the new version that I have made edits to. Is this an issue you have ever faced ??

@novemberborn
Copy link
Member Author

npm test calls npx test-ava which uses a different copy of AVA to test AVA with. But these tests are meant to be integration style, so you'd set up a fixture which uses the new syntax and then you observe the behavior.

@adiSuper94
Copy link
Contributor

I haven't worked on this for a while now. And I am facing troubles setting up the the test, so that the tests use the my changes. If some else has more experience doing I'd like some help, or they can take up the this issue entirely.

@arescrimson
Copy link

@adiSuper94 , I can take up this issue if you want. In addition, what troubles were you having setting up the tests?

@oantoro
Copy link
Contributor

oantoro commented Sep 26, 2024

I think one of the ways to implement this feature is to let the fn handler to return a new subsequence chain and the callWithFrag() must return it so that the next call to .because() function can be handled. The subsequence handler must be aware of the associated task/test created by the previous handler and also can modify its metadata. Or maybe there is easier approach to solve the problem?

@novemberborn
Copy link
Member Author

@oantoro IIRC that is how the chaining works, the challenge is probably that the "here's the test implementation" is the last step currently, so that needs to change.

@oantoro
Copy link
Contributor

oantoro commented Oct 14, 2024

I think additional logic is needed to modify the metadata after the given test is created.
So the function callWithFlag() might need some adjustment and also the handler needs some modification to return sub handler where the sub handler will modify the metadata of the associated test to include the error message when the .because(msg) function is called. The idea is to let callWithFlag() to return another chain so that the test.failing(title, cb).because(msg) call can be chained. Also when the task is successfully executed, the message must be emitted here. I think this approach would work but I am not sure whether the additional complexity is worthy or not.

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

No branches or pull requests

6 participants