Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix "deno test" on windows #599

Closed
wants to merge 10 commits into from
Closed

Fix "deno test" on windows #599

wants to merge 10 commits into from

Conversation

lucacasonato
Copy link
Member

Fixes #593

Use backslashes on windows for the default matcher globs.

Some tests don't run correctly on windows because they expect \n to result in a line break or / to be the file separator. These should probably be fixed in different PRs to keep this one simple.

@bartlomieju
Copy link
Member

It seems that the file matching is working fine after all (example build). Maybe let's put off this change until we land updated test runner. Tests will fail if there are no matching files found.

@lucacasonato
Copy link
Member Author

@bartlomieju The test succeedes, but it doesn't actually test anything. The tests itself aren't actually run. Look at the windows part of that link you sent. It finishes after just under 2 minutes while both Linux and MacOS take significantly longer. If you look at where 'deno test' is actually we excuted you will see that it does not find any tests to run. That means the tests on windows are broken. This is caused by the globs that are designed for Unix not working for windows because it uses a different path delimiter. I think this should go in as it fixes the issue.

@lucacasonato
Copy link
Member Author

It seems that the file matching is working fine after all (example build). Maybe let's put off this change until we land updated test runner. Tests will fail if there are no matching files found.

Oh and tests do not fail if no files are found.

@bartlomieju
Copy link
Member

@bartlomieju The test succeedes, but it doesn't actually test anything. The tests itself aren't actually run. Look at the windows part of that link you sent. It finishes after just under 2 minutes while both Linux and MacOS take significantly longer. If you look at where 'deno test' is actually we excuted you will see that it does not find any tests to run. That means the tests on windows are broken. This is caused by the globs that are designed for Unix not working for windows because it uses a different path delimiter. I think this should go in as it fixes the issue.

Ha! I could swear that I had different result when I pasted the link, dunno... Then the issue stands.

Oh and tests do not fail if no files are found.

Yes, they don't right now, I forgot to tag the PR which will introduce this behavior: #604

testing/runner.ts Outdated Show resolved Hide resolved
@nayeemrmn nayeemrmn mentioned this pull request Sep 19, 2019
@nayeemrmn
Copy link
Contributor

nayeemrmn commented Sep 19, 2019

@lucacasonato You'll have to fix the issues to get this green.

For getMatchingUrlsLocal() just use join() instead of / like you've done here.

For xevalCliReplvar() replace \n with EOL from //fs/path/constants.ts.

testing/runner_test.ts Outdated Show resolved Hide resolved
nayeemrmn pushed a commit to nayeemrmn/deno_std that referenced this pull request Sep 20, 2019
@nayeemrmn
Copy link
Contributor

nayeemrmn commented Sep 20, 2019

@lucacasonato On second thoughts, I've just applied these fixes in #604 (b3926f0).

It would probably be easier to close this 🙏

@lucacasonato
Copy link
Member Author

No Problem

nayeemrmn pushed a commit to nayeemrmn/deno_std that referenced this pull request Sep 21, 2019
nayeemrmn pushed a commit to nayeemrmn/deno_std that referenced this pull request Sep 21, 2019
nayeemrmn pushed a commit to nayeemrmn/deno_std that referenced this pull request Sep 26, 2019
inverted-capital pushed a commit to dreamcatcher-tech/napps that referenced this pull request Oct 24, 2024
This brings all session-related logic under one roof.
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.

[testing] deno test does not work on windows
3 participants