Skip to content

Conversation

@pke
Copy link

@pke pke commented Oct 26, 2019

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.

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.
@pke pke force-pushed the fix/spawn-without-quotes-on-windows branch from 1c75590 to b7a6c8d Compare October 26, 2019 23:46
@pke
Copy link
Author

pke commented Oct 26, 2019

@BrandonZacharie not sure why the tests would fail now. Could you take a look please?

@BrandonZacharie
Copy link
Owner

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.

@BrandonZacharie
Copy link
Owner

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.

@pke
Copy link
Author

pke commented Oct 28, 2019

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.

@pke
Copy link
Author

pke commented Nov 5, 2019

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.

@pke
Copy link
Author

pke commented Nov 10, 2019

@BrandonZacharie I've just run the tests on the master branch on a fresh linux machine. They fail with 3 tests.

image

So this has nothing to do with my patch for Windows. Could you please verify on your side again the tests are fine?

@BrandonZacharie
Copy link
Owner

@BrandonZacharie I've just run the tests on the master branch on a fresh linux machine. They fail with 3 tests.

image

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

@BrandonZacharie
Copy link
Owner

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.

@BrandonZacharie
Copy link
Owner

What version of Redis are you running on Linux?

@pke
Copy link
Author

pke commented Nov 13, 2019

Redis server v=5.0.3 sha=00000000:0 malloc=jemalloc-5.1.0 bits=64 build=afa0decbb6de285f

@pke
Copy link
Author

pke commented Nov 14, 2019

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants