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

134: add inventory service tests #87

Open
wants to merge 22 commits into
base: main
Choose a base branch
from

Conversation

JettTech
Copy link
Contributor

@JettTech JettTech commented Feb 28, 2025

Closes #134

NB: In the name of smaller PRs, I've separated out the tests from the main inventory PR. To review the inventory work itself, checkout out pr #86 .

@JettTech JettTech marked this pull request as ready for review March 1, 2025 00:11
@JettTech JettTech requested a review from a team as a code owner March 1, 2025 00:11
@JettTech JettTech changed the title add inventory tests 134: add inventory tests Mar 1, 2025
@JettTech JettTech changed the title 134: add inventory tests 134: add inventory service tests Mar 1, 2025
use super::*;

#[tokio::test]
async fn test_handle_authenticated_inventory_update() -> Result<()> {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can we rename the tests to this pattern
should_update_inventory_when_authenticated()
should_{action}_when_{state}

Copy link
Contributor Author

@JettTech JettTech Mar 1, 2025

Choose a reason for hiding this comment

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

I'm fine with that pattern, but we don't have any agreement about this yet and it is not a standard across this repo. I'm going to defer this particular request to another PR based on a team decision, as whatever pattern we decide on should be applied to all tests in this repo (and probably have a reciprocal pr in the holo-host-private).

@JettTech JettTech requested review from zeeshan595 and a team March 4, 2025 06:09
Base automatically changed from 134-add-inventory-sans-auth to main March 5, 2025 18:25
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