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

Using mocha for running some tests #2837

Closed

Conversation

StefanStojanovic
Copy link
Contributor

@StefanStojanovic StefanStojanovic commented May 4, 2023

Checklist
  • npm install && npm test passes
  • tests are included
  • commit message follows commit guidelines
Description of change

Constant GitHub Action failures started after landing 02480f6. One way to fix this is reverting that commit as proposed in #2849. This PR uses a different approach by keeping the make-fetch-happen version and making changes in testing modules.

The first thing updated is the tap module, from its legacy version 12.7.0 to the latest version 16.3.4. This change fixed the failing tests, but afterward, test-addon.js started failing randomly in GitHub Actions. Rerunning failed jobs would sometimes fix that, but in some cases, mostly on Windows, they stood broken after many reruns. There was no consistency in this and no pattern was to be found. On the other hand, running the npm test locally always passed for me and I tested with various node/python combinations.

Since I couldn't reproduce any issues locally, I decided to try changing the testing framework (at least for the failing tests). That brings me to my second change - using mocha to run test-addon.js which had to be adapted to the new framework. After doing this everything passed in the GitHub Actions without any issues.

Other ideas

At this time, the solution isn't pretty but it unblocks the CI, which was my main goal. Potentially, all tests can be migrated to mocha eventually, thus tap can be removed completely then. Another idea (proposed in #2851) is to simply use mocha to run tests we already have (eg. mocha test/test-*) instead of tap, and keep tests written in tap.

@StefanStojanovic StefanStojanovic marked this pull request as draft May 4, 2023 11:43
@StefanStojanovic StefanStojanovic changed the title Test Github actions constant failure investigation May 8, 2023
@cclauss cclauss changed the title Github actions constant failure investigation GitHub Actions constant failure investigation May 9, 2023
@StefanStojanovic StefanStojanovic force-pushed the mefi-github-actions-test branch 4 times, most recently from e186ce0 to 1247646 Compare May 11, 2023 16:02
@StefanStojanovic StefanStojanovic force-pushed the mefi-github-actions-test branch 10 times, most recently from d639602 to 52ae417 Compare May 15, 2023 14:45
@StefanStojanovic StefanStojanovic force-pushed the mefi-github-actions-test branch 4 times, most recently from a7e339a to 1a2445c Compare May 16, 2023 15:44
@legobeat
Copy link

@StefanStojanovic this may be relevant: #2848

legobeat added a commit to legobeat/node-gyp that referenced this pull request May 17, 2023
This reverts commit 02480f6, thereby
rolling back dependency make-fetch-happen from ^11.0.3 to ^10.0.3.

The upgrade is breaking for node-fetch users as it has transitive
dependencies with syntax incompatible with supported Node.js versions.

Related:
- nodejs#2770
- nodejs#2837
- nodejs#2816
- nodejs#2848
- nodejs#2827
- nodejs#2796
After make-fetch-happen update GitHub Actions started failing. Updating
tap to the latest version and migrating to mocha for addon tests fixes
GitHub Action failures.
@StefanStojanovic StefanStojanovic changed the title GitHub Actions constant failure investigation Using mocha for running some tests May 17, 2023
@StefanStojanovic
Copy link
Contributor Author

Closing in favor of #2851

legobeat added a commit to legobeat/node-gyp that referenced this pull request Jun 26, 2023
This reverts commit 02480f6, thereby
rolling back dependency make-fetch-happen from ^11.0.3 to ^10.0.3.

The upgrade is breaking for node-fetch users as it has transitive
dependencies with syntax incompatible with supported Node.js versions.

Related:
- nodejs#2770
- nodejs#2837
- nodejs#2816
- nodejs#2848
- nodejs#2827
- nodejs#2796
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