fix(gold-traces): detected launchers that run in cmd.exe on Windows - #12
Merged
Merged
Conversation
The record command runs through the platform's shell, which is cmd.exe on Windows. Three detected launchers did not run there: - venv: the file is appmap-python.exe, so the venv was never found; and a relative command path needs backslashes in cmd.exe - mvnw, gradlew: cmd.exe reads ./mvnw as the command `.` with a switch; the bare name works, since cmd.exe searches the current directory and adds the PATHEXT suffix itself - bin/rails: no extension, so it runs through ruby resolveRunner, buildCommand and planRecordCommands take a platform (default process.platform) so the tests cover Windows on any machine. One test runs the detected wrapper through the real shell, and on Windows also shows that ./mvnw does not start. Closes #8 Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
They describe the sh command line, and ran with the real platform, so on the Windows runner they got the cmd.exe forms. The Windows forms have their own tests. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
appmap-ruby's rspec and minitest hooks enable recording by themselves (DetectEnabled#enabled_by_testing? runs before the APPMAP check). APPMAP=true enables every recording method, requests included, and the gem warns about it. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #8.
The record command runs through the platform's shell, which is
cmd.exeon Windows. Three detected launchers did not run there..venv/bin/appmap-python ...(venv never found).venv\Scripts\appmap-python .venv\Scripts\pytestappmap-python.exe; a relative command path needs backslashes incmd.exe./mvnw testmvnw testcmd.exereads./mvnwas the command.with a switch. The bare name works:cmd.exesearches the current directory and adds thePATHEXTsuffix itself, so it findsmvnw.cmd../gradlew appmap testgradlew appmap testbin/rails testruby bin/rails testPATHEXTcannot helpNothing changes on macOS or Linux.
resolveRunner,buildCommandandplanRecordCommandstake aplatform(defaultprocess.platform), the waydetectMaxCommandLengthalready does, so the Windows cases are tested on any machine.One test runs the detected wrapper launcher through the real shell (
sh, orcmd.exeon the Windows runner). On Windows it also checks that./mvnw testdoes not start. That is the evidence for thecmd.execlaims in #8.🤖 Generated with Claude Code