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

feat(rust): add integration tests and some improvements #1883

Merged
merged 7 commits into from
Jun 22, 2024

Conversation

alexandreyc
Copy link
Contributor

@alexandreyc alexandreyc commented May 24, 2024

This should be the last PR for the initial Rust implementation.

It includes:

  • An integration test for the driver manager against the official SQLite driver
  • An integration test for the dummy driver against itself (comparing output when using the Rust API and when using its exported version through the driver manager)
  • New info codes and options which have been added recently
  • An enum for identifying statistics

Note that I also have an integration test for the driver manager against the official PostgreSQL driver but it needs a running PostgreSQL instance to be executed, so it cannot be easily integrated into the CI. This test proved useful during development as it allowed me to catch some bugs. The existence of this test is the reason why there is a common module. Maybe we can add it in a follow-up PR?

@mbrobbel
Copy link
Contributor

mbrobbel commented Jun 6, 2024

Anything I can do to help make progress here?

@alexandreyc
Copy link
Contributor Author

Sorry, I've been quite busy these past two weeks.

I just updated the description of the PR which should give you up-to-date informations.

I think you can start reviewing it!

@alexandreyc alexandreyc marked this pull request as ready for review June 6, 2024 08:01
Copy link
Contributor

@mbrobbel mbrobbel left a comment

Choose a reason for hiding this comment

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

Looks good to me.

rust/core/tests/common/mod.rs Show resolved Hide resolved
@lidavidm
Copy link
Member

Presumably that clang-tidy check doesn't need to be in the Rust pipeline? But that can be fixed separately

@lidavidm
Copy link
Member

Note that I also have an integration test for the driver manager against the official PostgreSQL driver but it needs a running PostgreSQL instance to be executed, so it cannot be easily integrated into the CI. This test proved useful during development as it allowed me to catch some bugs. The existence of this test is the reason why there is a common module. Maybe we can add it in a follow-up PR?

See the integration.yml pipeline which does this - can probably duplicate it for Rust or figure out how to work Rust into it

@lidavidm lidavidm merged commit aec0e8d into apache:main Jun 22, 2024
25 of 26 checks passed
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.

3 participants