-
-
Notifications
You must be signed in to change notification settings - Fork 58
chore: Added Switch as build target for integration tests
#2509
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
base: main
Are you sure you want to change the base?
Conversation
Semver Impact of This PR🟢 Patch (bug fixes) 📋 Changelog PreviewThis is how your changes will appear in the changelog. This PR will not appear in the changelog. 🤖 This preview updates automatically when you update the PR. |
| LogDebug("Project contains Sentry."); | ||
| } | ||
|
|
||
| var args = CommandLineArguments.Parse(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Duplicate command line argument parsing implementations
Low Severity
The new CommandLineArguments.Parse() class in SentrySetup.cs duplicates the existing Builder.ParseCommandLineArguments() method in Builder.cs. Both implementations have identical logic for parsing command line arguments into a Dictionary<string, string>. This introduces maintenance burden since bug fixes or improvements need to be applied to both places.
Additional Locations (1)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.
| Set-Content -Path $metaPath -Value $metaContent -NoNewline | ||
| } | ||
|
|
||
| Write-Host "Successfully copied $($files.Count) file(s) with meta files for platform '$Platform'" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Script reports success when zero files copied
Medium Severity
The copy-native-plugins.ps1 script reports "Successfully copied 0 file(s)" when the source directory exists but contains no files. Since this script's purpose is to provide required native plugin files for the Switch build, copying zero files is never valid, yet the script exits successfully. This causes the Unity build to fail later with an unclear error instead of failing immediately with a clear message about missing native files.


Summary
This aims to allow running
integration-test.ps1locally and on the console runner.The integration test consists of
Implementation
The
integration-test.ps1was originally designed to do what CI does, but locally, using the same scripts. This now extends this to support the manual copying of native libs step. For that reason building and packaging/extracting got stripped out intodev-integration-test.ps1to still allow for local workflows.Usage
Local Development
Integration Test (Phase 1: Building)
#skip-changelog