Skip to content

Conversation

@cspotcode
Copy link
Collaborator

@cspotcode cspotcode commented May 12, 2020

CI is split into 2 steps:

  • The first builds and packages ts-node, to closely match what we will eventually push to npmjs.com when we publish from a Posix machine.
  • The second runs tests, which pulls the package from the first step. Unfortunately we must build again because index.spec.ts needs to be compiled.

Getting tests to pass required the following changes to tests:

  • rather than exec node ./node_modules/.bin/ts-node, we must exec ./node_modules/.bin/ts-node which allows the windows .CMD shims to execute.
  • Change how CLI args are quoted and escaped in exec() calls to behave on Windows. For example, single-quoting is not handled by the shell.
  • compiler typeRoots use forward slashes on all platforms, so test assertions need to convert windows paths to have forward slashes
  • On Windows, only run symlink/realpath test when is actually a symlink. Git only creates symlinks if explicitly enabled.
  • On Windows, TS 2.7, always skip the 8.2.0 regression #884 regression test. It consistently fails and I don't know why.

And the following bugfix:

  • In ESM hooks, need to use fileURLToPath and pathToFileURL to manipulate file URLs. My previous approach was naive and not handling windows paths correctly.

@coveralls
Copy link

coveralls commented May 12, 2020

Coverage Status

Coverage decreased (-0.6%) to 80.552% when pulling f56f1b0 on ab/ci-tests-on-windows into 907935c on master.

@cspotcode cspotcode requested a review from blakeembrey May 13, 2020 06:20
@cspotcode cspotcode marked this pull request as ready for review May 13, 2020 06:20
@cspotcode cspotcode merged commit d339107 into master May 14, 2020
@cspotcode cspotcode mentioned this pull request May 15, 2020
@blakeembrey blakeembrey deleted the ab/ci-tests-on-windows branch May 15, 2020 04:22
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.

4 participants