Skip to content

Make generator path quoting test cross-platform - #1812

Merged
Jevan Saks (jevansaks) merged 6 commits into
mainfrom
user/jevansa/fix-generator-path-test
Sep 3, 2026
Merged

Make generator path quoting test cross-platform#1812
Jevan Saks (jevansaks) merged 6 commits into
mainfrom
user/jevansa/fix-generator-path-test

Conversation

@jevansaks

@jevansaks Jevan Saks (jevansaks) commented Sep 3, 2026

Copy link
Copy Markdown
Member

Summary

  • make the generator tool-path quoting test use platform-native separators
  • preserve coverage that paths containing spaces are quoted
  • run every portable test project in the GitHub Linux PR job, matching the official Linux build

Validation

  • dotnet test test\CsWin32Generator.BuildTasks.Tests -c Release --filter "TestCategory!=HighMemory&TestCategory!=RequiresHardware" --no-build
  • dotnet test test\GenerationSandbox.Tests -c Release --filter "TestCategory!=HighMemory&TestCategory!=RequiresHardware" --no-build
  • dotnet test test\GenerationSandbox.Unmarshalled.Tests -c Release --filter "TestCategory!=HighMemory&TestCategory!=RequiresHardware" --no-build

Fixes the Linux failure in official build 15191331 for version 0.3.332 and closes the PR-validation coverage gap that allowed it through.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 269885ab-a187-403f-81ef-4421f979d78f
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 269885ab-a187-403f-81ef-4421f979d78f
@jevansaks
Jevan Saks (jevansaks) marked this pull request as ready for review September 3, 2026 03:28
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 269885ab-a187-403f-81ef-4421f979d78f
@jevansaks
Jevan Saks (jevansaks) marked this pull request as draft September 3, 2026 03:46
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 269885ab-a187-403f-81ef-4421f979d78f
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 269885ab-a187-403f-81ef-4421f979d78f
@jevansaks
Jevan Saks (jevansaks) marked this pull request as ready for review September 3, 2026 04:31
@jevansaks
Jevan Saks (jevansaks) requested a balanced review from Copilot September 3, 2026 04:39
@jevansaks

Copy link
Copy Markdown
Member Author

Copilot is there a better way to get the Mono runtime on the machine for the tests that doesn't require "sudo" on the agent? Or is this the recommended way of doing it?

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟡 Changes recommended

The Linux CI installation can block awaiting confirmation unless apt-get install includes -y.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

Makes generator path-quoting tests cross-platform and expands Linux PR validation.

Changes:

  • Uses platform-native path separators in quoting tests.
  • Runs portable test projects on Linux with Mono support.
  • Mono installation must add -y to prevent CI from hanging or aborting.
File summaries
File Description
test/CsWin32Generator.BuildTasks.Tests/BuildTaskTests.cs Makes path-quoting assertions platform-neutral.
.github/workflows/build.yml Expands Linux test coverage; requires non-interactive Mono installation.
Review details
  • Files reviewed: 2/2 changed files
  • Comments generated: 1
  • Review effort level: Balanced

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread .github/workflows/build.yml Outdated

Copilot AI commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

Copilot is there a better way to get the Mono runtime on the machine for the tests that doesn't require "sudo" on the agent? Or is this the recommended way of doing it?

For GitHub-hosted ubuntu-latest, sudo apt-get is the recommended approach: the runner user has passwordless sudo, while APT installs system packages and cannot install Mono into the workspace as a normal user. Mono is not guaranteed to be preinstalled, so relying on it would be brittle. The alternatives are maintaining a self-hosted/prebaked runner image or skipping the Mono-dependent tests; neither is preferable here. The install should also be non-interactive (apt-get install -y ...).

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 269885ab-a187-403f-81ef-4421f979d78f
@jevansaks
Jevan Saks (jevansaks) marked this pull request as draft September 3, 2026 04:52
@jevansaks
Jevan Saks (jevansaks) marked this pull request as ready for review September 3, 2026 04:54
@jevansaks
Jevan Saks (jevansaks) enabled auto-merge (squash) September 3, 2026 05:03
- name: ⚙️ Install Mono
run: |
sudo apt-get update
sudo apt-get install -y --no-install-recommends mono-complete

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Should this be pinned to some target version? Or does the ubuntu-latest have the right inventory control?

@jevansaks
Jevan Saks (jevansaks) merged commit 1404ffd into main Sep 3, 2026
32 of 34 checks passed
@jevansaks
Jevan Saks (jevansaks) deleted the user/jevansa/fix-generator-path-test branch September 3, 2026 07:33
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.

5 participants