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

Upgrade dependencies #253

Merged
merged 5 commits into from
Nov 14, 2023
Merged

Upgrade dependencies #253

merged 5 commits into from
Nov 14, 2023

Conversation

svanherk
Copy link
Contributor

@svanherk svanherk commented Nov 8, 2023

With these changes:

  • Node 18 will now be required (everyone currently using @brightspace-ui/testing is on node 18).
  • For some reason, single quotes around the --files piece doesn't work (from a quick search, I don't think anyone is doing this except this repo)
  • Dev mode was turned on by default in the runner and the dev-server
    • We already turn off nodeResolve in the dev server for the report
    • I've changed the default in the runner to be { exportConditions: ['default'] }

@svanherk svanherk requested a review from a team as a code owner November 8, 2023 16:55
@svanherk
Copy link
Contributor Author

svanherk commented Nov 8, 2023

I'm going to test this in core, and we might want to wait until next week to merge this since I'm not confident it won't break something.

package.json Outdated
"test": "npm run lint && npm run test:bin && npm run test:server && npm run test:browser",
"test:bin": "mocha './test/bin/**/*.test.js'",
"test:browser": "npm run test:browser:other && npm run test:browser:ctor",
"test:browser:other": "npx d2l-test-runner --files './test/browser/**/*.test.js'",
"test:browser:other": "npx d2l-test-runner --files ./test/browser/**/*.test.js",
Copy link
Contributor

@bearfriend bearfriend Nov 8, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What was the issue with quotes for you? This can cause issues on POSIX system because it tries to pre-fill the wildcard and send in an array.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Error was:

Error: Could not find any test files with pattern(s): './test/browser/**/*.test.js',!**/node_modules/**/*
    at createTestSessions (C:\Desire2Learn\Instances\BrightspaceUI\testing\node_modules\@web\test-runner-core\dist\runner\createSessionGroups.js:55:19)
    at new TestRunner (C:\Desire2Learn\Instances\BrightspaceUI\testing\node_modules\@web\test-runner-core\dist\runner\TestRunner.js:32:115)
    at Object.startTestRunner [as start] (C:\Desire2Learn\Instances\BrightspaceUI\testing\node_modules\@web\test-runner\dist\startTestRunner.js:24:24)
    at process.processTicksAndRejections (node:internal/process/task_queues:95:5)
    at async runTests (file:///C:/Desire2Learn/Instances/BrightspaceUI/testing/bin/d2l-test-runner.js:47:2)

Maybe we need to trim the quotes off in the client?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Was that locally or in CI? I think I saw @dlockhart do this another way somewhere but I can't remember what it was. Escape the *, maybe?

Copy link
Contributor Author

@svanherk svanherk Nov 8, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Locally. We ran into something similar here, but that was a totally different library. Seems odd. BrightspaceUI/core#4222

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, not recalling "the other way".

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like escaped quotes work. Still not sure what exactly changed, but I assume this is what could be used to get around the POSIX system issue?

@svanherk
Copy link
Contributor Author

This causes no vdiff changes in core: BrightspaceUI/core#4247

@svanherk svanherk merged commit 7c725a0 into main Nov 14, 2023
2 checks passed
@svanherk svanherk deleted the Upgrade_depenedencies_in_testing branch November 14, 2023 18:23
@ghost
Copy link

ghost commented Nov 14, 2023

🎉 This PR is included in version 1.10.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

@ghost ghost added the released label Nov 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants