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: testing structure and outdated dependencies #248

Merged
merged 6 commits into from
Oct 1, 2024
Merged

Conversation

harbassan
Copy link
Contributor

@harbassan harbassan commented Sep 30, 2024

Describe the issue

Our testing workflow shows very inconsistent behavior when run both locally and in the pr pipeline.

Describe the solution

A lot of the dependencies we're using were outdated, e.g. mongoose, mongo-memory-server and jest, so I upgraded all of them and fixed the conflicts.

Importantly, that meant getting rid of the outdated esm package we were using to allow us to use es modules within node, which is now supported natively in the node version we are using. I also had to slightly change the babel config that's used for testing because jest's es modules support is still experimental.

Fixing all of this exposed some issues with our tests that weren't being caught before, so I made some minor changes to the some of the api routes and also the tests themselves to make sure everything passed.

Risk

Since alot was changed in terms of dependencies, there might be some difficulty with replicating on everyone's machines.

Definition of Done

  • Code peer-reviewed
  • Wiki Documentation is written and up to date
  • Unit tests written and passing
  • Integration tests written and passing
  • Continuous Integration build passing
  • Acceptance criteria met
  • Deployed to production environment

Reviewed By

Who reviewed your PR - for commit history once merged

@harbassan harbassan changed the title fix: make scenarios get all endpoint return with a consistent order fix: testing structure and outdated dependencies Oct 1, 2024
@harbassan harbassan marked this pull request as ready for review October 1, 2024 04:06
Copy link
Member

@wjin-lee wjin-lee left a comment

Choose a reason for hiding this comment

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

Looking really nice! Maybe us finally migrating away from legacy?!?
That said—as with a similar PR before, this is a massive PR that I will inevitably miss stuff in. In lieu of a testing framework and better practices, I will approve tentatively.

@harbassan harbassan merged commit f5960c6 into master Oct 1, 2024
5 checks passed
@harbassan harbassan deleted the unit-tests branch October 1, 2024 18:09
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