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

migrate sql-mapper, sql-openapi, sql-graphql to node:test #1827

Merged
merged 14 commits into from
Dec 23, 2023

Conversation

mateonunez
Copy link
Contributor

This PR follows the #1796 by migrating tests from tap to node:test.

I introduced a simple helper method match, which has the same behavior as the tap match assertion. This method is useful when you want to assert that two objects match the same structure. Unfortunately, the node match assertion only works well for regular expressions.

I also created a runner file to avoid concurrency problems (this runner and others will be dropped when the --test-concurrency flag will be available in this CI, Node v20.10 was released right now).

Migrated test

sql-mapper

  • cache.test.js
  • composite.test.js
  • create-connection.test.js
  • entity.test.js
  • entity_transaction.test.js
  • hooks.test.js
  • ignore.test.js
  • inserted_at_updated_at.test.js
  • mapper.test.js
  • no-primary-key.test.js
  • or.test.js
  • schema.test.js
  • updateMany.test.js
  • where.test.js

Copy link
Member

@mcollina mcollina left a comment

Choose a reason for hiding this comment

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

any reason why this is still in draft?

return true
}

module.exports.match = match
Copy link
Member

Choose a reason for hiding this comment

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

This is better to be put inside utils module, I suspect this would be used often.

@mcollina
Copy link
Member

(CI is failintg)

@mateonunez
Copy link
Contributor Author

This PR is still in draft as I'm trying to fix the CI.

I think the test contexts are not triggering the table cleanup actions correctly. I'll spend more time on this since these tests are one of the main parts of Platformatic.

@mateonunez
Copy link
Contributor Author

Any suggestions as to why snapshot tests (sql-openapi) fail? @mcollina

@mcollina
Copy link
Member

Will try to take a look!

Signed-off-by: mateonunez <mateonunez95@gmail.com>
Signed-off-by: mateonunez <mateonunez95@gmail.com>
Signed-off-by: mateonunez <mateonunez95@gmail.com>
Signed-off-by: mateonunez <mateonunez95@gmail.com>
Signed-off-by: mateonunez <mateonunez95@gmail.com>
Signed-off-by: mateonunez <mateonunez95@gmail.com>
@mateonunez mateonunez force-pushed the chore/migrate-sql-mapper-tests branch from fcbfa58 to bc311ae Compare December 2, 2023 14:38
Signed-off-by: Matteo Collina <hello@matteocollina.com>
@mateonunez
Copy link
Contributor Author

Hey @mcollina, sorry this PR has been in draft for a while but unfortunately I haven't had the time to devote to it yet... in the next few days I would like to pick it up again

Copy link

New dependencies detected. Learn more about Socket for GitHub ↗︎

Packages Version New capabilities Transitives Size Publisher
borp 0.4.2 None +0 23.3 kB matteo.collina

mateonunez and others added 5 commits December 23, 2023 09:42
Signed-off-by: mateonunez <mateonunez95@gmail.com>
Signed-off-by: Matteo Collina <hello@matteocollina.com>
Signed-off-by: mateonunez <mateonunez95@gmail.com>
Signed-off-by: mateonunez <mateonunez95@gmail.com>
Signed-off-by: mateonunez <mateonunez95@gmail.com>
@mateonunez mateonunez marked this pull request as ready for review December 23, 2023 11:49
Copy link
Member

@mcollina mcollina left a comment

Choose a reason for hiding this comment

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

lgtm

@mcollina mcollina changed the title refactor(test): remove tap from sql-mapper tests migrate sql-mapper, sql-openapi, sql-graphql to node:test Dec 23, 2023
@mcollina mcollina merged commit ef4e14e into platformatic:main Dec 23, 2023
84 of 85 checks passed
@mateonunez mateonunez deleted the chore/migrate-sql-mapper-tests branch December 23, 2023 13:30
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

2 participants