-
Notifications
You must be signed in to change notification settings - Fork 62
GPII-3844: Prevent system wallpaper corruption when setting high-contrast mode #772
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
Conversation
…rrent system wallpapers files
| "configure": [ | ||
| // Remove current system wallpaper files. | ||
| { | ||
| "type": "gpii.windows.rm", |
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.
I assume we will need mocks for this action in universal otherwise this solution will no longer pass through integration tests.
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.
Weirdly I was expecting this to fail, and the Windows to pass, the Windows one have failed with a spurious fail due to process killing timeout probably, and this have passed.
|
CI job passed: https://ci.gpii.net/job/universal-tests/1697/ |
|
!! Why did it pass! |
Have no clue, but it did. |
|
My expectation is that it will have passed because of this nonsense - https://github.com/GPII/universal/blob/master/gpii/node_modules/testing/src/Mocks.js#L197 - perhaps at some point when it is your normal working hours you could take a look at it :) |
|
@amb26 that looks like a promising start point! Sure I will, I would like to know better several places of the current testing infrastructure, so I can propose changes for improve some sections. This will be a nice exercise for that. |
|
All that we will need here is to set up some dedicated map of platform-specific functions to mocks, which can just be fluid.identity or a log function - otherwise in future someone will be Deeply Surprised by the fact that everything passes through a single hard-coded function with an expectation there is just one mocked function |
|
CI job failed: https://ci.gpii.net/job/universal-tests/1783/ |
…ests definition and implemented mocked function
|
CI job failed: https://ci.gpii.net/job/universal-tests/1784/ |
|
CI job failed: https://ci.gpii.net/job/universal-tests/1785/ |
| var fluid = fluid || require("infusion"), | ||
| gpii = fluid.registerNamespace("gpii"); | ||
| gpii = fluid.registerNamespace("gpii"), | ||
| nPath = require("path"); |
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.
Often we say "pathModule" for this kind of thing
…at are directly picked from SR
|
CI job passed: https://ci.gpii.net/job/universal-tests/1788/ |
testData/solutions/win32.json5
Outdated
| "type": "gpii.windows.rm", | ||
| "path": "${{environment}.APPDATA}\\Microsoft\\Windows\\Themes\\TranscodedWallpaper", | ||
| "options": { | ||
| "recursive": false, |
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.
Maybe you can remove this option since the default value for "recursive" option is "false" as seen here: https://github.com/GPII/windows/pull/261/files#diff-864b1fb848cfe2a0afe44120b1955365R1929
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.
Yes, doing it.
|
CI job passed: https://ci.gpii.net/job/universal-tests/1797/ |
|
@amb26 is this ready to go? |
No description provided.