Skip to content

fix(tests): pass the absolute path to the executable classifier - #345

Merged
bobtista merged 1 commit into
developmentfrom
fix/native-launch-test-build
Aug 1, 2026
Merged

fix(tests): pass the absolute path to the executable classifier#345
bobtista merged 1 commit into
developmentfrom
fix/native-launch-test-build

Conversation

@bobtista

@bobtista bobtista commented Aug 1, 2026

Copy link
Copy Markdown

development does not compile: #332 and #339 merged without a textual conflict, but #339 changed ExecutableFileClassifier.RequiresExecutePermission to require an absolutePath argument, leaving one stale call site in the tests #332 added.

All three CI jobs fail at Run Tests with:

NativeClientManifestLaunchTests.cs(138,57): error CS7036: There is no argument given that
corresponds to the required parameter 'absolutePath' of
'ExecutableFileClassifier.RequiresExecutePermission(string, string?)'

The loop already has the absolute path in hand, so it is passed through rather than switching to RequiresExecutePermissionFromName. That gives the magic-byte check #339 introduced, matching what ManifestGenerationService and ContentManifestBuilder do in production. This was the only remaining stale call site.

Greptile Summary

The PR repairs the native-client manifest launch tests after the executable classifier began requiring an absolute path.

  • Passes the enumerated file’s absolute path to ExecutableFileClassifier.RequiresExecutePermission.
  • Aligns the test manifest’s executable classification with production’s content-aware classification behavior.

Confidence Score: 5/5

The PR appears safe to merge, with the stale test call corrected to match the classifier’s current contract.

The enumerated file path is passed in the expected absolute-path parameter position, restoring compilation and enabling the intended magic-byte classification for extensionless native binaries.

Important Files Changed

Filename Overview
GenHub/GenHub.Tests/GenHub.Tests.Core/Features/GameProfiles/NativeClientManifestLaunchTests.cs Correctly supplies the full fixture file path to the executable classifier while retaining the filename for extension-based classification.

Reviews (1): Last reviewed commit: "fix(tests): pass the absolute path to th..." | Re-trigger Greptile

@coderabbitai

coderabbitai Bot commented Aug 1, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Summary by CodeRabbit

  • Bug Fixes
    • Improved executable file classification when launching native game profiles by evaluating each file’s complete path.

Walkthrough

Native manifest construction now passes each file’s full path to ExecutableFileClassifier.RequiresExecutePermission for path-aware executable classification.

Changes

Native manifest launch

Layer / File(s) Summary
Path-aware executable classification
GenHub/GenHub.Tests/.../NativeClientManifestLaunchTests.cs
The executable classification call now receives the file name and full path.

Estimated code review effort: 1 (Trivial) | ~2 minutes

Possibly related PRs

Suggested labels: Testing

Poem

A rabbit checks each file with care,
And sends its full path through the air.
The manifest knows what can run,
Classification work is done.
Hop, hop—tests now track the path!

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Title check ✅ Passed The title follows Conventional Commits syntax and accurately describes the test fix.
Description check ✅ Passed The description clearly explains the compilation failure and the absolute-path fix.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/native-launch-test-build

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot added the Testing Topic related to (unit) tests label Aug 1, 2026
@kilo-code-bot

kilo-code-bot Bot commented Aug 1, 2026

Copy link
Copy Markdown

Code Review Summary

Status: No Issues Found | Recommendation: Merge

Files Reviewed (1 file)
  • GenHub/GenHub.Tests/GenHub.Tests.Core/Features/GameProfiles/NativeClientManifestLaunchTests.cs

Reviewed by glm-5.2 · Input: 16.4K · Output: 2.7K · Cached: 174.7K

@bobtista
bobtista merged commit e626088 into development Aug 1, 2026
8 checks passed
@bobtista
bobtista deleted the fix/native-launch-test-build branch August 1, 2026 19:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Testing Topic related to (unit) tests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant