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

Error on Windows as of 1.62.993 #270

Open
skynet-gh opened this issue Feb 16, 2022 · 9 comments
Open

Error on Windows as of 1.62.993 #270

skynet-gh opened this issue Feb 16, 2022 · 9 comments
Assignees
Labels
bug Something isn't working regression

Comments

@skynet-gh
Copy link

I'm getting this error now when running kaocha on my project:

> clj -M:kaocha
java.nio.file.NotDirectoryException: tests.edn
 at sun.nio.fs.WindowsWatchService$Poller.implRegister (WindowsWatchService.java:379)
    sun.nio.fs.AbstractPoller.processRequests (AbstractPoller.java:266)
    sun.nio.fs.WindowsWatchService$Poller.run (WindowsWatchService.java:596)
    java.lang.Thread.run (Thread.java:833)
[watch] watching stopped.

deps.edn alias and tests.edn

Looks like the file watcher changes are causing it to think tests.edn should be a directory for some reason. Adding :kaocha.watch/type :hawk does not fix it.

@oxalorg
Copy link
Member

oxalorg commented Feb 16, 2022

:kaocha.watch/type :hawk does not fix it

Hmmmm! This is weird. Because using that flag it should have reverted back to the old watching mechanism. Maybe there has been an unintended change in this release? 🤔

Thanks for reporting this issue I personally don't use windows so it might be hard to debug this! Any help would be appreciated.

At the risk of asking the obvious, does the immediate last release of Kaocha work fine for you?

@skynet-gh
Copy link
Author

:kaocha.watch/type :hawk does not fix it

Hmmmm! This is weird. Because using that flag it should have reverted back to the old watching mechanism. Maybe there has been an unintended change in this release? 🤔

Actually, I was wrong, I was putting :kaocha.watch/type :hawk in the test config and not top-level. Putting it top-level works!

At the risk of asking the obvious, does the immediate last release of Kaocha work fine for you?

It does not but I'll use the workaround of hawk for now, thanks!

@plexus plexus added bug Something isn't working regression labels Mar 28, 2022
@alysbrooks
Copy link
Member

Looking at the Beholder repository, it looks like Beholder supports Windows, so I'm not sure why this failed. Based on the error, it could be a bug in the WindowsWatchService. In that case, it may be a version-specific issue.

@alysbrooks
Copy link
Member

So Beholder doesn't support file-level watching, which we try to do for tests.edn. Apparently the WindowWatchService notices, and the ones for Linux and macOS don't (and instead silently fail). There's a couple options:

  • Watch the whole project directory and filter out events we don't want. The main issue here is that if you're doing ClojureScript builds or other operations that touch a lot of files, there's a lot of events to filter out.
  • Poll. I normally wouldn't consider this, but we would be polling a very small number of files, so it might be okay.
  • Add some sort of file-level watching upstream. Apparently, the operating system-level file watch functionality is directory-based, so this might be an uphill battle on the JDK or io.methvin.watcher level. I could see NextJournal going for it, so I might reach out to them. The code would be fairly similar whether it goes in Kaocha or Beholder so any design or coding wouldn't be wasted either.

@plexus
Copy link
Member

plexus commented Sep 20, 2022

Looking at the code I think we should revisit this implementation, it starts two watchers, one is given all the test-paths/source-paths, the other is given #{config-file}, i.e. tests.edn. I think instead we can use a single watcher at the project top-level, and then filter for the stuff we care about.

Slightly different use case but there's some code here that can server as inspiration. It takes a set of directories, and reduces it to the minimum set by removing directories that are children of other directories already included in the set. https://github.com/lambdaisland/launchpad/blob/main/src/lambdaisland/launchpad/watcher.clj

So here it'll be similar

  • start from (concat test-paths source-paths [project-root])
  • remove any redundant paths (most of the time all that will be left is [project-root], but there could be test/source paths outside of the project root
  • watch those paths
  • filter events by checking again against the initial list ((concat test-paths source-paths [project-root])) (see Path#startsWith

@alysbrooks alysbrooks moved this from 📋 Information Needed to Candidate in Lambda Island Open Source Sep 21, 2022
@colin-p-hill
Copy link

So Beholder doesn't support file-level watching, which we try to do for tests.edn. Apparently the WindowWatchService notices, and the ones for Linux and macOS don't (and instead silently fail).

Since it sounds like watching the config file doesn't currently work on any platform, would it be possible to get an interim band-aid of just removing this block? This would get rid of the exception on Windows, and Linux and Mac wouldn't suffer for it because their watchers are failing silently anyway.

@alysbrooks
Copy link
Member

@colin-p-hill That's not a bad idea. I think we'd probably make it Hawk-only instead of removing it, though.

@alysbrooks
Copy link
Member

alysbrooks commented Jan 10, 2023

@skynet-gh @colin-p-hill I just merged a temporary fix to this issue similar to what Colin suggested. You can point Kaocha to the latest changeset 7d6c897bfd85fd66aea0c45fcbb62de37733f374 or wait until we publish a new release.

{lambdaisland/kaocha {:git/url "https://github.com/lambdaisland/kaocha"
                      :git/sha "7d6c897bfd85fd66aea0c45fcbb62de37733f374"}}

@alysbrooks
Copy link
Member

I forgot to mention it at the time, but you can grab the latest version with the temporary fix (among other improvements):

[lambdaisland/kaocha "1.78.1249"]
{lambdaisland/kaocha {:mvn/version "1.78.1249"}}

@alysbrooks alysbrooks moved this from Candidate to 🏗 In progress in Lambda Island Open Source Feb 23, 2023
@alysbrooks alysbrooks self-assigned this Jun 17, 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 regression
Projects
Status: 🏗 In progress
Development

No branches or pull requests

5 participants