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

Use startup_file to force a standardized repos option during testing #650

Merged
merged 2 commits into from
Dec 3, 2024

Conversation

DavisVaughan
Copy link
Contributor

Because r-lib/rig#203 forces a non-@CRAN@ default for my CRAN repo, meaning that the tests don't pass for me locally because apply_repo_defaults() sees that something is set and doesn't override it.

#[cfg(windows)]
#[cfg(not(unix))]
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I also tweaked this because we use #[cfg(unix)] above, so it seems cleanest to use not(unix) to ensure we dont miss anything

Copy link
Contributor

Choose a reason for hiding this comment

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

Makes sense!

Comment on lines +50 to +61
assert_eq!(frontend.recv_shell_execute_reply(), input.execution_count);

let code = r#"getOption("repos")[["Internal"]]"#;
frontend.send_execute_request(code, ExecuteRequestOptions::default());
frontend.recv_iopub_busy();

let input = frontend.recv_iopub_execute_input();
assert_eq!(input.code, code);
assert_eq!(
frontend.recv_iopub_execute_result(),
r#"[1] "https://internal.cran.mirror/""#
);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added testing for the Internal repo too to make sure it gets set

@DavisVaughan DavisVaughan requested a review from jmcphers December 3, 2024 14:47
Copy link
Contributor

@jmcphers jmcphers left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for this!

Confirmed tests pass locally with this change, too.

running 1 test
test test_auto_repos ... ok

test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.42s

     Running tests/repos-conf-file.rs (target/debug/deps/repos_conf_file-eca1d963ef415763)

running 1 test
test test_conf_file_repos ... ok

test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.43s

#[cfg(windows)]
#[cfg(not(unix))]
Copy link
Contributor

Choose a reason for hiding this comment

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

Makes sense!

@DavisVaughan DavisVaughan merged commit 0281acd into main Dec 3, 2024
6 checks passed
@DavisVaughan DavisVaughan deleted the feature/tweak-cfg branch December 3, 2024 22:17
@github-actions github-actions bot locked and limited conversation to collaborators Dec 3, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants