Improve "Project does not exists" popup

kiskoza commented 8 months ago

Identify the Bug

This is a first improvement to fix #833.

Description of the Change

I've changed the warning to be dismissable (won't auto-hide anymore) and added two buttons. The first one "Remove all", removes all missing folders from the project. The second one just closes the pop-up for now.

Alternate Designs

On the ticket there were some discussions if we can add an option to fix the paths or remove only some of them. I think just removing all of the deleted folders solves the problem most of the time, I would not write this right now.

There's a reason behind adding a "Snooze" button, something like we already have for new versions' popup. I'm happy to add it, if you also see the need for this.

Possible Drawbacks

Without the snooze button, the auto-hiding popup will became a permanent popup on every start.

Verification Process

Follow the steps described on the issue.

Release Notes

The missing folder popup warning now has a button to remove the missing folders from the workspace.

Other things to discuss

[ ] The function I extended with the new logic is already a huge one, I'd like to move the missing folder handling out to a new function. As I'm not a regular maintainer, I don't know if you have a policy about it (e.g.: don't do this because xy reason, follow some naming convention, etc)
[ ] The extended tests are failing for me on local. I'm not entirely sure why, as most of the jasmine docs indicates that the any helper should work with the old jasmine version we're using here.

kiskoza commented 8 months ago

The CI passed, but it has a silent fail (the same I got on my local)

ALL TESTS THAT FAILED:
AtomEnvironment ::openLocations(locations) when there is no saved state when the opened path does not exist may be required to be an existing directory

Finished in 443.882 seconds
2289 tests, 13116 assertions, 1 failure, 2 skipped

error Command failed with exit code 1.
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.
*********************
Tests failed. Retrying failed tests...
*********************

yarn run v1.22.22
$ electron --no-sandbox --enable-logging . -f --test spec
[18428:0314/090207.681379:ERROR:bus.cc(393)] Failed to connect to the bus: Could not parse server address: Unknown address type (examples of valid types are "tcp" and on UNIX "unix")
[18456:0314/090207.694566:ERROR:viz_main_impl.cc(150)] Exiting GPU process due to errors during initialization
[18428:0314/090207.768734:INFO:CONSOLE(13)] "(electron) The remote module is deprecated. Use https://github.com/electron/remote instead.", source: electron/js2c/renderer_init.js (13)
[18428:0314/090207.816183:INFO:CONSOLE(113)] "%cElectron Security Warning (Insecure Content-Security-Policy) font-weight: bold; This renderer process has either no Content Security
    Policy set or a policy with "unsafe-eval" enabled. This exposes users of
    this app to unnecessary security risks.

For more information and help, consult
https://electronjs.org/docs/tutorial/security.
This warning will not show up
once the app is packaged.", source: electron/js2c/renderer_init.js (113)
[18428:0314/090207.828468:INFO:CONSOLE(33)] "The vm module of Node.js is deprecated in the renderer process and will be removed.", source: electron/js2c/renderer_init.js (33)
[18428:0314/090208.102230:INFO:CONSOLE(33)] "The vm module of Node.js is deprecated in the renderer process and will be removed.", source: electron/js2c/renderer_init.js (33)
[18428:0314/090208.282855:INFO:CONSOLE(33)] "The vm module of Node.js is deprecated in the renderer process and will be removed.", source: electron/js2c/renderer_init.js (33)
(node:18480) Electron: Loading non-context-aware native module in renderer: '/home/runner/work/pulsar/pulsar/node_modules/pathwatcher/build/Release/pathwatcher.node'. This is deprecated, see https://github.com/electron/electron/issues/18397.
(Use `exe --trace-warnings ...` to show where the warning was created)
(node:18480) Electron: Loading non-context-aware native module in renderer: '/home/runner/work/pulsar/pulsar/node_modules/superstring/build/Release/superstring.node'. This is deprecated, see https://github.com/electron/electron/issues/18397.
(node:18480) Electron: Loading non-context-aware native module in renderer: '/home/runner/work/pulsar/pulsar/node_modules/tree-sitter/build/Release/tree_sitter_runtime_binding.node'. This is deprecated, see https://github.com/electron/electron/issues/18397.
(node:18480) Electron: Loading non-context-aware native module in renderer: '/home/runner/work/pulsar/pulsar/node_modules/git-utils/build/Release/git.node'. This is deprecated, see https://github.com/electron/electron/issues/18397.
[18428:0314/090208.682460:ERROR:bus.cc(393)] Failed to connect to the bus: Could not parse server address: Unknown address type (examples of valid types are "tcp" and on UNIX "unix")

Finished in 0.022 seconds
0 tests, 0 assertions, 0 failures, 2291 skipped

Done in 1.42s.

DeeDeeG commented 7 months ago

Regarding the "silent fail":

I'm trying to debug the script/run-tests.js script.

This script notices that some error occurred, so the failing tests should be retried, but it uses an empty list of tests to to filter which ones to retry. Then, since there are no "failing tests" in the "re-run" step (100% of tests are skipped which technically gives a "passing" exit code not "failing")... the "empty re-run" pass exits with a passing/clean exit code and the CI run "succeeds".

confused-Techie commented 7 months ago

@DeeDeeG Thats an alarming discovery. We should probably insure that if script/run-tests.js tries to do a run with zero tests, it will always fail, since I'd rather a false negative than a false positive in our CI

pulsar-edit / pulsar