search

gotestyourself / gotestsum

'go test' runner with output optimized for humans, JUnit XML for CI integration, and a summary of the test results.
Apache License 2.0
2.03k stars 119 forks source link

watcher: warning on invalid symlinks #370

Closed smoynes closed 11 months ago

smoynes commented 11 months ago

When watching for filesystem events, gotestsum will report a failure to follow a symlink as a warning. 

$ gotestsum --watch &
Watching 1 directories. Use Ctrl-c to to stop a run or exit.
$ ln -s "/not/a/file" .#file.go
$ fg
WARN failed to put terminal (fd 0) into raw mode: interrupted system call
WARN failed to stat .#file.go: stat .#file.go: no such file or directory

The first warning makes sense to me: it is an artifact of the reproduction script putting gotestsum in the background. However, the second warning seems a bit distracting or, at least, unnecessary.

As I understand it, when handling a file-system event, the handler gets the status of the modified dirent, checks if it a new directory and, if so, adds it to the watch set. When encountering a symlink to a non-existent file, the handler gets the status of the target of the link, finds the target is nonsense, and gets os.ErrNotExist.

In this case, the symlink is indeed not a directory and so should not be added to the watch set. Tf the symlink is later changed to an extant directory, the watcher will get another change event and can start watching the new (valid) directory.

If this seems worth fixing to the maintainers, I am happy to contribute a better test case or a change. I think the handler could use lstat to check if the file is a link and silence the warning.

An alternative might be to ignore hidden files. It would be more complicated, but go build already ignores them and arguably the test runner ought to as well. There are probably side effects to this that aren't obvious, though.

(It is curious where these files come from: my editor creates temporary non-existent symlinks as part of its file-locking protocol. More distractingly, it happens when I first edit the file, not when I save it, so it isn't when one would expect tests to run. While I can configure the editor not to create locks or to put them elsewhere, I think the issue of invalid symlinks is more general -- more a systemic issue in Unix -- and is somewhat independent of weird file locking by my weird editor.)

dnephin commented 11 months ago

Thanks for the bug report! I agree it should be possible to hide these not-actionable warnings. I think changing that log line from Warnf to Debugf would be a good way to hide it.

The "failed to put terminal into raw mode" error could probably be improved as well. It would probably be more helpful as:

terminal not available - key shortcuts disabled

That way users get an indication that the rest of --watch is working normally.

If you are interested in PR'ing one or both of these changes it would be appreciated!

smoynes commented 11 months ago

I appreciate the opportunity to contribute to a thing I use almost every day. Thanks! 🌞

I'll look into improving the raw mode message, as well.