Skip to content
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 need to save twice to trigger watch on OSX using Neovim #300

Conversation

andersjanmyr
Copy link
Contributor

On OSX (12.6.2) using Neovim (NVIM v0.8.2) the triggering event varies between fsnotify.Rename and Create.
This fix adds fsnotify.Rename to the events triggering a new test run.

Before

handling event "cors_middleware.go": RENAME
handling event "cors_middleware.go": CREATE

Running tests in ./.
exec: [go test -json ./.]
go test pid: 96495
✓  . (cached)

DONE 71 tests in 0.237s

After

handling event "cors_middleware.go": RENAME

Running tests in ./.
exec: [go test -json ./.]
go test pid: 95707
✓  . (cached)

DONE 71 tests in 0.245s

On OSX (12.6.2) using Neovim (NVIM v0.8.2) the triggering event varies
between `fsnotify.Rename` and `Create`.
This fix adds `fsnotify.Rename` to the events triggering a new test run.

*Before*
```
handling event "cors_middleware.go": RENAME
handling event "cors_middleware.go": CREATE

Running tests in ./.
exec: [go test -json ./.]
go test pid: 96495
✓  . (cached)

DONE 71 tests in 0.237s
```

*After*
```
handling event "cors_middleware.go": RENAME

Running tests in ./.
exec: [go test -json ./.]
go test pid: 95707
✓  . (cached)

DONE 71 tests in 0.245s
```
@dnephin dnephin added the bug Something isn't working label Feb 5, 2023
@dnephin
Copy link
Member

dnephin commented Feb 6, 2023

Thank you for the bug fix! This is definitely something that needs to be fixed.

I think this change should be safe because of the throttling of events, but I'd like to do a bit of testing with vim, vs code, and GoLand to make sure it continues to work correctly with those editors.

The changes in this PR look great, so as long as this continues to work with those editors I think we should be able to merge this PR.

Copy link
Member

@dnephin dnephin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for fixing this! These changes work well for me. The throttling of events prevents tests from being run extra times.

@dnephin dnephin merged commit f39e7fa into gotestyourself:main Feb 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants