node-redis-server
node-redis-server copied to clipboard
BrandonZacharie
fix: Do not quote spawn args on Windows
Without setting windowsVerbatimArguments: true in the
spawn options the redis-server would be spawned with
redis-server.exe "--port 3679" which it would not accept as a valid argument.
@BrandonZacharie not sure why the tests would fail now. Could you take a look please?
I had Travis CI run tests on the development branch again since they had not not been run in over a year to make sure they still pass and they do.
I see two things are needed here. First, I suppose windowsVerbatimArguments should only equal true when running under Windows. Second, a Travis CI configuration that will run tests under Windows in addition to Linux.
Thanks for looking into this. According to the nodejs docs the windowsVerbatimArguments argument is ignored on anything but Windows, so it should be safe.
I'll add a Travis CI config entry for Windows.
Hmmm, adding Windows CI target did not really help. It checks out the code with CRLF endings and the linter already errors.
And the tests that do run, have the same timeout problem as in linux env.
But I fail to see how my change causes the tests to fail.
@BrandonZacharie I've just run the tests on the master branch on a fresh linux machine. They fail with 3 tests.
So this has nothing to do with my patch for Windows. Could you please verify on your side again the tests are fine?
@BrandonZacharie I've just run the tests on the master branch on a fresh linux machine. They fail with 3 tests.
So this has nothing to do with my patch for Windows. Could you please verify on your side again the tests are fine?
It looks like your tests are failing because that port is taken; I assume Redis is already running
The tests pass for me locally and pass in CI. There may be dragons but I haven't been able to reproduce errors except when changing windowsVerbatimArguments.
Redis server v=5.0.3 sha=00000000:0 malloc=jemalloc-5.1.0 bits=64 build=afa0decbb6de285f
Running this on a system with no redis running from the master branch results in the exact 3 failed tests. I am out of ideas. Which redis are you testing with? @BrandonZacharie