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

Support message testing function in throws & throwsAsync assertions #2995

Merged
merged 7 commits into from May 16, 2022

Conversation

il3ven
Copy link
Contributor

@il3ven il3ven commented Mar 26, 2022

Fixes #2978

As per the issue #2978, I have added support for function in throws/throwsAsync assertion. I have also updated the docs regarding the same. I couldn't find any tests to be updated so I have left them.

How to use?

await t.throwsAsync(async () => {
	throw new TypeError('🦄');
}, {
	message: message => ['🦄', '🐴'].includes(message)
});

Note: This is my first PR in ava so I may have missed something. Please let me know. I will fix it. For example, I am not sure about the assertion message to show in case of failure.

docs/03-assertions.md Outdated Show resolved Hide resolved
docs/03-assertions.md Outdated Show resolved Hide resolved
lib/assert.js Outdated Show resolved Hide resolved
@sindresorhus
Copy link
Member

You also need to update the TypeScript types.

The commit includes changes in docs, `instaceOf Function` and TS types.
@il3ven il3ven requested a review from sindresorhus April 3, 2022 18:24
types/assertions.d.ts Outdated Show resolved Hide resolved
@novemberborn
Copy link
Member

@il3ven the code looks great, but it seems some tests are not passing.

@il3ven
Copy link
Contributor Author

il3ven commented Apr 19, 2022

@novemberborn I have fixed the test in test-tap/assert.js. The pipeline also passes now. However there is one test that fails in my local system. I could not debug the issue with that test. The same test also fails in the main branch. Does this also happen on your system? If no, then we can forget about the issue and move on.

Steps to reproduce

  • Run npx tap test-tap/api.js
  • Observe the assert at line#154 fail in case of opts.workerThread == true
Error output in my system
$ npx tap test-tap/api.js 

​ FAIL ​ test-tap/api.js
 ✖ should be equal

  --- expected    
  +++ actual      
  @@ -1,1 +1,1 @@ 
  -1              
  +2              

  test: "test-tap/api.js fail-fast mode - workerThreads: true - multiple files &
    interrupt"
  at:
    line: 154
    column: 5
    file: file:///Users/vaibhav/repos/il3ven/temp-ava/test-tap/api.js
    type: Test
  stack: |
    Test.<anonymous> (file://test-tap/api.js:154:5)

​ FAIL ​ test-tap/api.js 1 failed of 128 1m
 ✖ should be equal

​

                         
  🌈 SUMMARY RESULTS 🌈  
                         
​ FAIL ​ test-tap/api.js 1 failed of 128 1m
 ✖ should be equal

​
Suites:   ​1 failed​, ​1 of 1 completed​
Asserts:  ​​​1 failed​, ​127 passed​, ​of 128​
​Time:​   ​1m​

My Machine
OS: MacOS 12.3.1
Node: v14.18.2

@novemberborn
Copy link
Member

However there is one test that fails in my local system. I could not debug the issue with that test.

Yea I think that test is flaky, no worries.

@novemberborn
Copy link
Member

@il3ven sorry I was AFK for nearly two weeks :) Test pass now, but it strikes me there's no test for the new behavior?

@il3ven
Copy link
Contributor Author

il3ven commented May 2, 2022

@il3ven sorry I was AFK for nearly two weeks :)

No problem.

Test pass now, but it strikes me there's no test for the new behavior?

I will add them. Can you provide the files where these tests should be added? Or maybe point me to existing tests for the throwsAsync function.

@novemberborn
Copy link
Member

IIRC it's all in https://github.com/il3ven/ava/blob/throws/test-tap/assert.js#L915=.

@il3ven
Copy link
Contributor Author

il3ven commented May 3, 2022

@novemberborn I have added test cases to cover the code that I added. I have also added a few more cases to cover similar code which was uncovered before.

I haven't added a test case for throwsAsync because the same code is being used both in throws and throwsAsync. Let me know if I need to add it separately.

@novemberborn
Copy link
Member

@il3ven this is great!

The additional test cases would be nice to communicate intent, but then again it's a massive test file that we haven't ported to be self-hosted, so I'm not too concerned about that.

@novemberborn novemberborn changed the title add support for function in throws/throwsAsync assertion Support message testing function in throws & throwsAsync assertions May 16, 2022
@novemberborn novemberborn merged commit e387cba into avajs:main May 16, 2022
@il3ven il3ven deleted the throws branch May 16, 2022 08:50
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.

Support function in message expectation in t.throws / t.throwsAsync
3 participants