这是indexloc提供的服务,不要输入任何密码
Skip to content

ndc-test nix improvements and Chinook data fixes #43

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

Merged
merged 8 commits into from
Apr 17, 2024

Conversation

daniel-chambers
Copy link
Contributor

@daniel-chambers daniel-chambers commented Apr 17, 2024

This PR improves some of the testing infrastructure, but stops short of actually integrating ndc-test in CI. We still need to make improvements to ndc-test and fix more bugs in the mongo connector before that will be possible.

Nix

The Nix arion testing configuration was modified slightly to support mounting the snapshots directory in the repo and reading and writing ndc-test snapshots to/from that directory. There are some example commands of how to do this commented out in project-ndc-test.nix. However, it's still configured to just do a basic "test" run because:

  • snapshots currently just create broken tests; for example, ordering tests are generated using unstable orderings, so the snapshots always fail
  • snapshots generated in an unusable fashion, where they are just sprayed into the folder under folders named after the hash of the requests. This is very unhelpful, but will require improvements to ndc-test to change.

Chinook data

The Chinook dataset had some significant issues, which have now been resolved! 😄 Namely:

  • Many tables failed to import correctly from CSV due to type issues. This has been resolved by changing the import process so that it imports from JSON files instead of CSV. The JSON files were manually exported after manually massaging the CSV data into Mongo correctly and then exporting it as JSON.
  • Importing from CSV every time meant that new object IDs were generated each time the import was performed. This is incompatible with snapshot testing, so now that we're importing from JSON, the Object IDs have been captured and don't change on every import.
  • Some of the data types in the schema were incorrect, namely any column that dealt with money was being typed as a float, when it really should have been a decimal. The Employee.ReportsTo column was also a string, when it should have been an int. These issues have also been corrected.

JIRA: MDB-110

@daniel-chambers daniel-chambers self-assigned this Apr 17, 2024
@daniel-chambers daniel-chambers marked this pull request as ready for review April 17, 2024 07:53
Copy link
Collaborator

@hallettj hallettj left a comment

Choose a reason for hiding this comment

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

Looks great, thanks for all of these improvements! We still need to update the fixture ddn to close the loop. I don't think that's a blocker for the PR since technically everything is working as is. I may get the CLI running for real to work on that.

configuration-dir = ../fixtures/connector/chinook;
database-uri = "mongodb://mongodb:${mongodb-port}/chinook";
service.depends_on.mongodb.condition = "service_healthy";
# Run the container as the current user so when it writes to the snapshots directory it doesn't write as root
service.user = builtins.toString config.host.uid;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Hey that's handy!

@daniel-chambers daniel-chambers merged commit 93e0b09 into main Apr 17, 2024
@daniel-chambers daniel-chambers deleted the daniel/ndc-test branch April 17, 2024 23:59
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