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

Fix failing tests #385

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open

Fix failing tests #385

wants to merge 5 commits into from

Conversation

JKRhb
Copy link
Member

@JKRhb JKRhb commented Apr 21, 2024

This PR will investigate and hopefully eventually solve the issues we currently observe when it comes to testing the library.

@coveralls
Copy link

coveralls commented Apr 21, 2024

Pull Request Test Coverage Report for Build 9616729684

Details

  • 22 of 24 (91.67%) changed or added relevant lines in 2 files are covered.
  • 2 unchanged lines in 1 file lost coverage.
  • Overall coverage decreased (-0.1%) to 91.636%

Changes Missing Coverage Covered Lines Changed/Added Lines %
lib/server.ts 17 19 89.47%
Files with Coverage Reduction New Missed Lines %
lib/server.ts 2 86.22%
Totals Coverage Status
Change from base Build 9363797542: -0.1%
Covered Lines: 2893
Relevant Lines: 3111

💛 - Coveralls

@JKRhb
Copy link
Member Author

JKRhb commented Apr 21, 2024

Apparently, it's only two tests that are broken under Node 20. I will look into how to solve the problem here.

@Apollon77
Copy link
Collaborator

overlaps replaces #362 ?

@JKRhb
Copy link
Member Author

JKRhb commented Apr 21, 2024

overlaps replaces #362 ?

Hmm, good point! Maybe updating will actually solve the issue here – I will test it out :)

@JKRhb JKRhb force-pushed the fix-failing-tests branch 2 times, most recently from 5626498 to 3e53f98 Compare June 21, 2024 14:57
@JKRhb JKRhb force-pushed the fix-failing-tests branch 3 times, most recently from f3eda87 to bcef9b0 Compare June 21, 2024 15:46
@JKRhb JKRhb force-pushed the fix-failing-tests branch 2 times, most recently from 8e202b6 to cae5bb1 Compare June 21, 2024 15:47
@JKRhb JKRhb marked this pull request as ready for review June 21, 2024 16:22
@JKRhb JKRhb requested a review from Apollon77 June 21, 2024 16:22
@JKRhb
Copy link
Member Author

JKRhb commented Jun 21, 2024

I think this PR is now ready for review :) The tests are passing now again (I also added Node 22 to the test matrix) – however, I needed to skip a few of them since there seem to be some changes in Node 20 and above that cause issues with sinon.

In general, the tests are somewhat flaky at the moment, but I think we can probably deal with that in future PRs. For now, it is probably more important to be able to move forward again (and maybe also release a new version).

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.

None yet

3 participants