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 cli on tests #102

Merged

Conversation

joaoantoniocardoso
Copy link
Member

@joaoantoniocardoso joaoantoniocardoso commented Nov 14, 2024

This will allow us to fix the test errors introduced by #101.

The problem happens when a test code calls any of the functions of the cli module, which then makes the lazy_static to create cli::Args from the commandline of the binary being executed, conflicting with the test harness/setup.

This change allows us to define the parsed cli::Args so that it won't conflict with the arguments being passed to the test harness/setup.

.. yeah, this is a drawback of singletons using lazy_static.

src/lib/cli.rs Outdated
#[instrument(level = "debug")]
pub fn log_path() -> String {
let log_path =
MANAGER.clap_matches.log_path.clone().expect(
MANAGER.get().unwrap().clap_matches.log_path.clone().expect(
Copy link
Member

Choose a reason for hiding this comment

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

can we have a macro to make the access easier ?

Copy link
Member Author

Choose a reason for hiding this comment

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

I made it into a local function instead =)

@patrickelectric patrickelectric merged commit 106bc6d into bluerobotics:master Nov 14, 2024
8 checks passed
@joaoantoniocardoso joaoantoniocardoso deleted the fix_cli_on_tests branch November 14, 2024 16:45
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