-
Notifications
You must be signed in to change notification settings - Fork 3k
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 all tests on Windows and add run-script for Walkontable watch (#5878) #6187
Conversation
by changing single quotes to escaped double quotes, because otherwise I got the following error: 'np' is not recognized as an internal or external command, operable program or batch file. 'run' is not recognized as an internal or external command, operable program or batch file. The filename, directory name, or volume label syntax is incorrect. '--watch'' is not recognized as an internal or external command, operable program or batch file. 'np' is not recognized as an internal or external command, operable program or batch file. 'run' is not recognized as an internal or external command, operable program or batch file. The filename, directory name, or volume label syntax is incorrect. '--watch'' is not recognized as an internal or external command, operable program or batch file.
analogous to "test:e2e.watch", but observes src and test changes for Walkontable
because on Windows there the viewport suitable for rendering is different, caused by the scrollbar size taller by 2px compared to macOS
…indows and macOS because the current value (150) causes the 9th row to be only visible on Windows. This, in turn does not trigger more rows to be rendered after 9th row. So we have 9 rendered rows on Windows and 16 on macOS With this change, there are 16 rows rendered on Windows and macOS
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
as noted in #6187 (review) apparently, CTRL+C on Windows sends a SIGQUIT, which was not handled also, bumped NPM packed Concurrently to its current version
I have addressed all comments, except of this one:
I have noticed it too, but I don't know what to do about it :( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 👍
Context
Continuation of #6128. This fixes #5878
I need this change, because I am using a Windows desktop PC for development. That's because
time npm run build
takes 53s on Windows and 1m21s on macOS.How has this been tested?
Run the following scripts on Windows with success:
As well as manually running spec runners in Chrome:
Types of changes
Related issue(s):
Checklist: