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

Fix: Move application integration tests to app crate so they actually run #326

Merged
merged 3 commits into from
Apr 28, 2023

Conversation

actioninja
Copy link
Contributor

@actioninja actioninja commented Apr 20, 2023

I'm submitting a

  • bug fix
  • feature
  • documentation addition

What is the current behaviour?

Integration tests don't run

What is the expected behavior?

Integration tests should run

Turns out that the dev dependency removal in #324 was slightly erroneous. Integration tests weren't in the proper directory so were never actually running or picked up as integration tests. This made cargo-udeps give a false positive for them.

This wasn't that big of a deal, considering the one integration test intended to test whether help printed but never actually did that.

Also moves utils.rs to a folder module, since they aren't picked up by the integration test scanner. the run command was changed to not strip out the initial "comtrya" since I found this a confusing behavior until I checked the implementation.

Copy link
Contributor

@martintc martintc left a comment

Choose a reason for hiding this comment

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

LGTM. Everything seems to build fine, I see more tests popping up when I run. Good catch! I'll give it a day or two for the other maintainers to pipe in if they get a chance. Otherwise, I'll merge it pending they don't pull the brakes on something.

@martintc martintc merged commit b716375 into comtrya:main Apr 28, 2023
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.

None yet

2 participants