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: support jest test #366

Merged

Conversation

pebo
Copy link
Contributor

@pebo pebo commented Mar 1, 2023

avoid parallel import() as this may trigger esm import bugs which blocks jest tests

Initial checks

  • Have you followed the guidelines in our Contributing document?
  • Have you checked to ensure there aren't other open Pull Requests for the same update/change?
  • Have you linked an issue to this pull request? (Create one if it does not exist)
  • Have you used Conventional Commits format?

[ISSUE TYPE]

#342

Description

See:

Implementation details

Perform lay loading of ./index_old.js (oasOld). This avoid the issue with parallel imports.

To verify with jest:

# install jest
npm install -D jest

# add a file jest smoke test in the file: /tests/jest.test.js
import express from 'express';
import oasTools from '@oas-tools/core';
import fs from 'fs';

test("jest test", async () => {
    const app = express();
    const defaults = JSON.parse(fs.readFileSync('tests/testServer/.oastoolsrc'));

    await oasTools.initialize(app, defaults);
});

# add a jest script to package.json
"jest-test": "NODE_OPTIONS=--experimental-vm-modules jest --env=node /tests/jest.test.js",

# run test
npm run jest-test

avoid parallel import() as this may trigger esm import bugs
@coveralls
Copy link

Pull Request Test Coverage Report for Build 4301579220

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 91.937%

Totals Coverage Status
Change from base Build 4287066458: 0.0%
Covered Lines: 889
Relevant Lines: 943

💛 - Coveralls

@alesancor1
Copy link
Member

Hey @pebo, could you take a look at #363 and see if that solves the issue with jest?

However, as I mention there, TS support is planned so I will be working on that line, which means the entrypoint.js and the index_old.js will be probably deleted

@pebo
Copy link
Contributor Author

pebo commented Mar 1, 2023

Hey @pebo, could you take a look at #363 and see if that solves the issue with jest?

I think that issue was about running jest with esm modules which works fine in general but there's a bug triggered when performing import() function calls in parallel where the same sub-module is loaded twice.

However, as I mention there, TS support is planned so I will be working on that line, which means the entrypoint.js and the index_old.js will be probably deleted

TS support sounds nice but would it make sense to release a 3.0.4 version before that with the current fixes on the develop branch and possible this PR (to solve the jest issue with parallel imports)?

@alesancor1
Copy link
Member

alesancor1 commented Mar 2, 2023

I see your point, but the CI/CD will bump the version directly to v3.1.0 due to some feats that have been merged recently. However, I may wait and leave TS for 3.2.0 so I can have more time for it, so yes, if I end up doing that I will merge this.

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