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 service tests on windows; run [crates] (to prove service tests still run) #8786

Merged
merged 2 commits into from
Jan 14, 2023

Conversation

chris48s
Copy link
Member

@chris48s chris48s commented Jan 7, 2023

Refs #8437

This doesn't fix the :trace variant but it does fix being able to run npm run test:services -- only="myservice" on Windows.

The core fix here is actually:

diff --git a/core/base-service/loader.js b/core/base-service/loader.js
index 9bbf0c948..5fd87cd87 100644
--- a/core/base-service/loader.js
+++ b/core/base-service/loader.js
@@ -103,7 +103,7 @@ async function collectDefinitions() {
 async function loadTesters() {
   return Promise.all(
     glob
-      .sync(path.join(serviceDir, '**', '*.tester.js'))
+      .sync(toUnixPath(path.join(serviceDir, '**', '*.tester.js')))
       .map(async path => await import(`file://${path}`))
   )
 }

(basically in #8350 we missed the tests loader).

My original plan was to do that and directly write tests for loadTesters() and loadServiceClasses() but loading all the tester classes with .map(async path => await import(`file://${path}`)) inside an expect() was causing some strange side effects with chai, so I decided to separate discovering all the files from loading them and write a test for just making sure we discover the files, which is why there is a bit more refactoring going on in this PR.

Side note: This doesn't have any bearing on anything, but I am unreasonably annoyed with myself, windows users, and the world as a whole that I had to install Windows in a VirtualBox in order to work this out when it basically amounted to applying a fix I had already worked out previously without having to do that.

@chris48s chris48s added bug Bugs in badges and the frontend core Server, BaseService, GitHub auth, Shared helpers labels Jan 7, 2023
@shields-ci
Copy link

shields-ci commented Jan 7, 2023

Messages
📖 ✨ Thanks for your contribution to Shields, @chris48s!

Generated by 🚫 dangerJS against f823f73

@chris48s chris48s changed the title WIP fix service tests on windows WIP fix service tests on windows; run [crates] (to prove service tests still run) Jan 7, 2023
@chris48s chris48s changed the title WIP fix service tests on windows; run [crates] (to prove service tests still run) fix service tests on windows; run [crates] (to prove service tests still run) Jan 7, 2023
@chris48s chris48s marked this pull request as ready for review January 7, 2023 22:23
@chris48s chris48s added the developer-experience Dev tooling, test framework, and CI label Jan 7, 2023
Copy link
Member

@calebcartwright calebcartwright left a comment

Choose a reason for hiding this comment

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

LGTM. Might be worth setting up a recurring CI check with Actions too

@github-actions
Copy link
Contributor

Messages
📖 ✨ Thanks for your contribution to Shields, @chris48s!

Generated by 🚫 dangerJS against 6896c65

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Bugs in badges and the frontend core Server, BaseService, GitHub auth, Shared helpers developer-experience Dev tooling, test framework, and CI
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants