Minor bug fixes #8
Conversation
…as a space) This wraps the karma base config in quotes if it's found to contain spaces. E.g., [2021-10-28 12:18:55.410] [DEBUG] [KarmaCommandLineTestServerExecutor:CommandLineProcessHandler]: Process 9y9frhllqbh: Executing command: 'npx' with args: ["karma","start","\"C:/Users/Jane Doe/.vscode/extensions/lucono.karma-test-explorer-0.2.1/dist/karma.conf\"","--no-single-run"]
…ge reload!" Experienced with karma 5.1.1 and jasmine 3.5.0
peitschie
force-pushed
the
executor-with-spaces
branch
from
bb3d62d
to
31d1769
Compare
|
Thanks for the PR, @peitschie. You mind opening bug issues for these two and providing the debug logs and as much other information as possible in the bug template - such as your operating system where this bug occurs. Command argument quoting and/or escaping has different rules on different operating systems and can easily work on one and break on another, being very error-prone. With the logs, it'll be easier to test and validate the fix for other platforms as well. Thanks. |
|
No worries at all. Can appreciate how complex multi-platform escaping is. I've tried to target the patch in a way that it will never make things worse, but I haven't really tried hard on OSX/Linux, as I know there are a lot more characters that can make their ways into paths on those systems. |
|
I've raised #9 for the spaces-in-paths issue, and #10 for the For the client context tweak however, I'm unable to find a reproduction case I can share publicly, unfortunately. I've been through my tests with a fine-tooth comb and found no issues. I've even tried stripping down the whole suite to practically nothing, and still this error turns up. As best as I can guess, it might be connected with karma-runner/karma#3482. Updating to karma 6 solves the issue, but for various reasons this isn't something I can do for the repo at this stage. I had a search through your own changes here, and could discover any reason why the |
|
@peitschie Thanks for filing the issues. The fix for issue #10 looks good, and I'll merge it if you can split out the proposed fixes for #9 into a different PR. I will look into the Mac/Linux part to make sure that the file path space issue can be resolved for all of the platforms after which I'll publish both fixes to the marketplace. Thanks. |
|
Apologies for the delay here @lucono ... it's been a long weekend for most in my country 😃 |
|
No worries @peitschie. I've included your proposed change for You can build it locally with |
|
Superseded by #11 🎉 |
Thanks for this fork! It's nice to see some activity around this again.
I've just fixed two minor issues that blocked me from running this extension with my local setup (karma+jasmine+webpack).
I'm happy to rework them in any way you see fit 😃