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

filemanager: setup testing #75

Merged
merged 23 commits into from
Dec 14, 2023
Merged

filemanager: setup testing #75

merged 23 commits into from
Dec 14, 2023

Conversation

mmalenic
Copy link
Member

@mmalenic mmalenic commented Dec 13, 2023

Closes #71

Changes

  • Adds database and unit tests.
  • Adds AWS test mocks using mockall.
  • General refactor to increase clarify and update docs.
  • Fix ordering and equality bug when de-duplicating and sorting events within the same Lambda function.

It should be a lot easier to test locally now just by running cargo test. This will mean that dev deployments are more likely to succeed if the tests pass locally. Now, if only there is way to remove docker compose up as well. Just being able to run cargo test directly would be the most convenient.

inner: ConfigLoader,
}

#[automock]
Copy link
Member Author

Choose a reason for hiding this comment

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

This creates an entirely new MockConfig struct which has methods that allow setting expectations.

@@ -0,0 +1,246 @@
#[double]
Copy link
Member Author

@mmalenic mmalenic Dec 13, 2023

Choose a reason for hiding this comment

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

This expands to:

#[cfg(not(test))]
use crate::clients::aws::s3::Client;
#[cfg(test)]
use crate::clients::aws::s3::MockClient as Client;

Which allows using different versions of the client in testing code compared to the main code.

@mmalenic mmalenic requested a review from brainstorm December 13, 2023 06:09
Copy link
Member

@brainstorm brainstorm left a comment

Choose a reason for hiding this comment

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

Very nice use of mockall and mock_double, I didn't know those crates, kudos Marko!

@mmalenic
Copy link
Member Author

Do you want to go for #74 first? There might be some conflicts.

@brainstorm
Copy link
Member

Do you want to go for #74 first? There might be some conflicts.

Ok, I'll put it up for review (now it's in draft), but there's still things left to fix :_S

# Conflicts:
#	lib/workload/stateful/filemanager/Cargo.lock
#	lib/workload/stateful/filemanager/README.md
#	lib/workload/stateful/filemanager/filemanager-http-lambda/src/main.rs
#	lib/workload/stateful/filemanager/filemanager-ingest-lambda/Cargo.toml
#	lib/workload/stateful/filemanager/filemanager/src/events/aws/mod.rs
#	lib/workload/stateful/filemanager/filemanager/src/events/s3/collect.rs
#	lib/workload/stateful/filemanager/filemanager/src/events/s3/s3_client.rs
@mmalenic mmalenic force-pushed the test/setup branch 3 times, most recently from c342188 to 0aee1e4 Compare December 14, 2023 07:15
@mmalenic mmalenic merged commit 6e94e42 into main Dec 14, 2023
@mmalenic mmalenic deleted the test/setup branch December 14, 2023 07:27
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.

filemanager: setup testing mocks
2 participants