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: switch from mocha and chai to the jest #353

Merged
merged 9 commits into from
Oct 17, 2022

Conversation

magicmatatjahu
Copy link
Member

@magicmatatjahu magicmatatjahu commented Sep 21, 2022

Description

  • switch from mocha and chai to the jest. Simple reason. Mocha and chai (specifically, a ts-node which transpiles code from TS to JS) has a some problems with importing ESM. I found that problems when I started writing tests for feat: integrate new parser-js #347 and new parser uses new module syntaxes. We still need chai in devDependencies, because @oclif/test uses it underneath.
  • bump @oclif/core to the latest ^1.18.0 version.

Related issue(s)
See more #347

Copy link
Member

@derberg derberg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sorry but let us first merge #221

@magicmatatjahu
Copy link
Member Author

@derberg Done, could you check again? :)

@magicmatatjahu
Copy link
Member Author

@derberg ping, ping :P

Copy link
Member

@derberg derberg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you share why changes in [src/base.ts](https://github.com/asyncapi/cli/pull/353/files#diff-440b55a9ff9e0486020fc49197f3278d14446306fc0c5d5107b48139c20bfda6) were needed? and also is oclif upgrade necessary, related to refactoring of tests?

@magicmatatjahu
Copy link
Member Author

@derberg When I switched to the jest, oclif doesn't recognized errors from stderr but it throws to the stdout, so I log errors by this.logToStderr(${e.name}: ${e.message}); line.

Related to the bump of oclif - I made a bump along with the transition to the jest. Between 1.11 and 1.18 there is nothing big that will cause a breaking change in our cli.

derberg
derberg previously approved these changes Oct 17, 2022
Copy link
Member

@derberg derberg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm, but because prod dependency got updated, we should release it as fix: and indicate it clearly in the description that it is not only refactor of tests really, but core dependency got also upgraded.

I approve but please make sure to make proper adjustments before merge

@magicmatatjahu magicmatatjahu changed the title refactor: switch from mocha and chai to the jest fix: switch from mocha and chai to the jest Oct 17, 2022
@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@magicmatatjahu
Copy link
Member Author

/rtm

@asyncapi-bot asyncapi-bot merged commit 2d37d1a into asyncapi:master Oct 17, 2022
@magicmatatjahu magicmatatjahu deleted the switch-to-jest branch October 17, 2022 12:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request ready-to-merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants