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 and upgrade test suite #25

Merged
merged 6 commits into from
Jan 24, 2020
Merged

Conversation

roydejong
Copy link

@roydejong roydejong commented Jun 6, 2019

When running yarn test on Windows 10 x64, with Node v10.15.3, the current version of node-pcsclite produces the following error:

TypeError: stub(obj, 'meth', fn) has been removed, see documentation

This causes all tests to fail on my system. This is related to a deprecation / API change in Sinon:
sinonjs/sinon#1761 (comment)

Some other things I've noticed:

  • The "start" and "connect" tests fail due to a timeout (they seem to be incomplete).
  • There is a Buffer deprecation warning (as of Node v10).
  • Mocha seems to hang after running the tests instead of exiting cleanly.

This PR addresses issues with the test suite by:

  • Applying sinon-codemod/extract-calls-fake to test.js.
  • Upgrading the sinon dependency in package.json to ^7.3.2 (latest release).
  • Updating the new Buffer() calls to Buffer.from().
  • Rewriting the "#start()" test so it makes sense, with sample data based on the example in pcsc.js.
  • Modifying the get_reader() test helper so it uses the correct end marker for reader names (fixes the "#connect()" test).
  • Tests: Add explicit done() calls all round to ensure the tests actually complete as expected.

@roydejong roydejong changed the title Tests fix: Update Sinon, apply sinon-codemod/extract-calls-fake Fix and upgrade test suite Jun 6, 2019
Rewriting the "#start()" test so it makes sense, with sample data based on the example in `pcsc.js`.
Modifying the `get_reader()` test helper so it uses the correct end marker for reader names (fixes the "#connect()" test).
@roydejong
Copy link
Author

roydejong commented Jun 6, 2019

It took a bit of work, but this PR is in a state where all the tests now pass successfully on my system, and everything works as expected.

There's still a bug where mocha doesn't exit on its own. It seems PCSCLite.close() isn't enough to shut everything down cleanly, but I'm not sure how to deal with that. Maybe someone else knows. (Note, it is possible to force mocha to exit anyway after tests with --exit flag)

@pokusew pokusew merged commit 9335a6c into pokusew:master Jan 24, 2020
@pokusew
Copy link
Owner

pokusew commented Jan 24, 2020

Hi @roydejong,

Thank you very much for your PR. I really appreciate the work you did. 👍

I am really sorry it look me so long to review and merge it.

Thanks again.

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.

2 participants