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

chore: Added tests for edge and client_api #98

Merged
merged 3 commits into from
Mar 8, 2023

Conversation

chriswk
Copy link
Member

@chriswk chriswk commented Mar 8, 2023

Also added some Tarpaulin exclusion arguments to better reflect code coverage across code that we actually want to cover.

There's still some work to do with regards to having the possibility to instantiate the entire application as a test service for full integration/e2e tests, but this at least covers more logic and also exposed a bug in how we validated keys when not using a token validator (offline mode))

Also added some Tarpaulin exclusion arguments to better reflect code
coverage across code that we actually want to cover.

There's still some work to do with regards to having the possibility to
instantiate the entire application as a test service for full
integration/e2e tests, but this at least covers more logic and also
exposed a bug in how we validated keys when not using a token validator
(offline mode))
.map(|features| features.clone())
.map(|client_features| ClientFeatures {
Copy link
Member

Choose a reason for hiding this comment

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

YESSS. Let's also make sure this goes into the commit message as a fix

ClientFeatures {
version: 2,
features: vec![
ClientFeature {
Copy link
Member

Choose a reason for hiding this comment

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

I think we have a Default impl on ClientFeature. Entirely optional but it might make this huge blob easier to read

@@ -0,0 +1,2 @@
[all]
exclude-files= ["src/main.rs", ""
Copy link
Member

Choose a reason for hiding this comment

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

YEET IT. YEET IT WITH FIRE

Copy link
Member

@sighphyre sighphyre left a comment

Choose a reason for hiding this comment

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

This is great, sets my heart at ease

@chriswk chriswk merged commit 036ab11 into main Mar 8, 2023
@chriswk chriswk deleted the task/addTestsForRemainingServiceEndpoints branch March 8, 2023 12:52
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