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

get the tests passing on windows #8350

Merged
merged 5 commits into from
Aug 29, 2022
Merged

get the tests passing on windows #8350

merged 5 commits into from
Aug 29, 2022

Conversation

chris48s
Copy link
Member

@chris48s chris48s commented Aug 28, 2022

closes #8289

Now that we are running on GHA we can pretty easily just run the tests on Windows and we've got tests covering the functionality in #8289

@chris48s chris48s added bug Bugs in badges and the frontend developer-experience Dev tooling, test framework, and CI labels Aug 28, 2022
@shields-cd shields-cd temporarily deployed to shields-pr-8350 August 28, 2022 19:51 Inactive
@shields-ci
Copy link

shields-ci commented Aug 28, 2022

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

Generated by 🚫 dangerJS against d546e61

@shields-cd shields-cd temporarily deployed to shields-pr-8350 August 28, 2022 19:55 Inactive
@chris48s
Copy link
Member Author

d313935 includes the fix suggested in #8289 (comment) which gets most of the tests passing. There are still a few more failures I've not looked into yet.

For the moment I've just started with the core tests.

@shields-cd shields-cd temporarily deployed to shields-pr-8350 August 29, 2022 12:03 Inactive
@chris48s chris48s changed the title WIP get the tests passing on windows get the tests passing on windows Aug 29, 2022
@chris48s chris48s marked this pull request as ready for review August 29, 2022 12:04
@chris48s
Copy link
Member Author

Right.. rest of the tests should be fixed, mysteries solved.

The reason for the behaviour change with paths (relatively) recently is due to a behaviour change in glob when we upgraded from 7.2 to 8.0: https://github.com/isaacs/node-glob/blob/main/changelog.md#80

@@ -1,5 +1,7 @@
function svg2base64(svg) {
return `data:image/svg+xml;base64,${Buffer.from(svg).toString('base64')}`
return `data:image/svg+xml;base64,${Buffer.from(svg.trim()).toString(
Copy link
Member Author

@chris48s chris48s Aug 29, 2022

Choose a reason for hiding this comment

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

The difference in the logo tests was a \n vs \r\n issue. I decided that really the best thing to do was just strip any kind of trailing whitspace rather than try to normalise it as any trailing whitspace after the final > is doing nothing of use anyway so 🤷

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.

Nice 👍 and agreed with the changes; line endings are a pain

@chris48s
Copy link
Member Author

Cool. It may also be worth running some of the other test suites on Windows as well as linux - these kind of issues could happen in the frontend/package/etc too, but also I'm slightly wary of "build matrix inflation" (for want of a better term). Interested if you have any thoughts on that point..

@shields-cd shields-cd temporarily deployed to shields-pr-8350 August 29, 2022 19:50 Inactive
@repo-ranger repo-ranger bot merged commit b624179 into master Aug 29, 2022
@repo-ranger repo-ranger bot deleted the 8289-windows2 branch August 29, 2022 19:58
@calebcartwright
Copy link
Member

but also I'm slightly wary of "build matrix inflation" (for want of a better term). Interested if you have any thoughts on that point..

Yeah I know what you mean, and it's especially true in my experience with GHA because of how granularly each leg shows up under the check status view.

I think my main interest for Windows is largely just to give us back a high degree of confidence that contributors on Windows can clone + install, then start the app, I'm much less concerned with having an exhaustive/equal testing suite on Windows

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 developer-experience Dev tooling, test framework, and CI
Projects
None yet
Development

Successfully merging this pull request may close these issues.

joi library startup issue
4 participants