remove unused imports, make clicktests more reliable, fix git rev-parse with bare repositories, fix crash when directory gets deleted #1263
Conversation
remove ensureAuthenticated from testing/cleanup
error event handler is required to not crash the process
|
It looks like git version 2.25 changed the behavior of That's the reason why travis with the latest git version and all appveyor builds are failing. Appveyor updated the git version a few days ago. |
campersau
force-pushed
the
unused_imports
branch
from
4275300 to
4a90cb0
Compare
| }); | ||
| app.post(`${exports.pathPrefix}/testing/shutdown`, ensureAuthenticated, (req, res) => { | ||
| res.json({}); | ||
| process.exit(); |
There was a problem hiding this comment.
Exiting the process during a request / response can result in connection reset errors on the client which can make tests fail.
| app.post(`${exports.pathPrefix}/testing/shutdown`, ensureAuthenticated, (req, res) => { | ||
| res.json({}); | ||
| process.exit(); | ||
| app.post(`${exports.pathPrefix}/testing/cleanup`, (req, res) => { |
There was a problem hiding this comment.
Removed ensureAuthenticated from this endpoint so it can be called during the AUTHENTICATION clicktests.
There was a problem hiding this comment.
Are these endpoints still available when running ungit outside of tests?
If it doesn't require authentication, should it be hidden behind a TEST env variable for example?
nmclicktests/environment.js
Outdated
| if (!doNotClose) { | ||
| return this.nm.end(); | ||
| if (this.ungitServer) { | ||
| this.ungitServer.kill('SIGINT'); |
There was a problem hiding this comment.
Instead of exiting the process from inside, the child process gets killed from the outside. (See above for reasons.)
| @@ -80,7 +81,11 @@ exports.registerApi = (env) => { | |||
| emitGitDirectoryChanged(socket.watcherPath); | |||
| emitWorkingTreeChanged(socket.watcherPath); | |||
| } | |||
| })); | |||
| }); | |||
| watcher.on('error', (err) => { | |||
There was a problem hiding this comment.
Listening for watcher errors is required otherwise the ungit process crashes when errors occur.
Steps to reproduce (on windows at least I got a permission error):
- initialize a new git repro in
some folder - navigate to
some folderin ungit - remove
some folder
campersau
force-pushed
the
unused_imports
branch
from
54c0ba6 to
dba7f02
Compare
campersau
changed the title
(also want to check timeouts in appveyor builds)
fixes #1265
fixes #1266