Skip to content

fix(ci): CodeQL-friendly session shell tests#109

Merged
d0dg3r merged 1 commit into
mainfrom
fix/codeql-session-test-split
Apr 8, 2026
Merged

fix(ci): CodeQL-friendly session shell tests#109
d0dg3r merged 1 commit into
mainfrom
fix/codeql-session-test-split

Conversation

@d0dg3r
Copy link
Copy Markdown
Owner

@d0dg3r d0dg3r commented Apr 8, 2026

Problem

GitHub CodeQL Rust analysis reported a warning on session.rs (macro expansion / panic_2021) when a single #[test] mixed #[cfg(not(target_os = "windows"))] and #[cfg(target_os = "windows")] assert! branches.

Change

Split builds_local_shell_command_from_explicit_shell into:

  • builds_local_shell_command_from_explicit_shell_posix (#[cfg(not(target_os = "windows"))])
  • builds_local_shell_command_from_explicit_shell_windows (#[cfg(target_os = "windows")])

Behavior is unchanged; cargo test session::tests passes locally.

CodeQL's Rust extractor warns on assert! macro expansion when POSIX and
Windows cfg branches live in the same test function. Use separate #[test]
items with #[cfg] on each function.
Copilot AI review requested due to automatic review settings April 8, 2026 01:06
@d0dg3r d0dg3r merged commit f03437a into main Apr 8, 2026
5 checks passed
@d0dg3r d0dg3r deleted the fix/codeql-session-test-split branch April 8, 2026 01:07
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR adjusts Rust unit tests in the Tauri desktop app’s session.rs to avoid a GitHub CodeQL Rust analysis warning caused by mixing #[cfg(...)] branches inside a single #[test].

Changes:

  • Split builds_local_shell_command_from_explicit_shell into two OS-specific tests using #[cfg(not(target_os = "windows"))] and #[cfg(target_os = "windows")].
  • Add a short explanatory comment about the CodeQL extractor behavior to justify the split.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

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