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

Seems broken on PyPy + Windows #792

Closed
oprypin opened this issue May 9, 2021 · 9 comments
Closed

Seems broken on PyPy + Windows #792

oprypin opened this issue May 9, 2021 · 9 comments

Comments

@oprypin
Copy link
Contributor

oprypin commented May 9, 2021

I tried to apply "watchdog" to a project here: mkdocs/mkdocs#2385

It works fine for everything, but on Windows PyPy3.6 the watches seem to silently produce no events.

Then I saw that dbc5689 kinda just removed PyPy from CI.

I launched a CI run with PyPy added back: https://github.com/oprypin/watchdog/actions/runs/825872726
And indeed, watchdog's own tests are failing just a well.

So, is PyPy supposed to be supported still?

@BoboTiG
Copy link
Collaborator

BoboTiG commented May 10, 2021

Well, PyPy is incredibly slow on Windows 😮
I added back PyPy CI in #794, and tests failures seems related that is is very slow. Compare a normal run (45 sec) to a PyPy run (15 minutes), that's horrible.

PyPy is known to be slow with ctypes. There may be room for improvements I guess.

To be complete, the old CI was testing PyPy on GNU/Linux only, it was never tested on Windows.

@oprypin
Copy link
Contributor Author

oprypin commented May 10, 2021

I don't think it's right to assume that the tests fail because they are slow. I would assume the opposite - that they are slow only because they fail (with the symptom being a timeout). Some tests succeed only occasionally, and when they do, they're very fast then.

@oprypin
Copy link
Contributor Author

oprypin commented May 10, 2021

So I added a sleep here

diff --git a/tests/test_emitter.py b/tests/test_emitter.py
index 708a0378b..f44aff602 100644
--- a/tests/test_emitter.py
+++ b/tests/test_emitter.py
@@ -88,6 +88,7 @@ def start_watching(path=None, use_full_emitter=False, recursive=True):
         emitter.suppress_history = True
 
     emitter.start()
+    time.sleep(0.01)
 
 
 def rerun_filter(exc, *args):

And it makes the tests pass just fine and quickly.

What this shows us is that

  1. The problem seems to be that modifications that happen instantly after starting a watch don't get detected.
  2. My comment (with the argument against your comment) was fully correct.

@oprypin
Copy link
Contributor Author

oprypin commented May 10, 2021

@BoboTiG
Copy link
Collaborator

BoboTiG commented May 11, 2021

I stand corrected, thanks for the investigation.
Do you mind opening a PR with your fix + mine + changelog entry?

@oprypin
Copy link
Contributor Author

oprypin commented May 11, 2021

Uh sorry, what I have there is not a fix. The library really has a flaw, and adding a sleep into tests would only conceal it.

@BoboTiG
Copy link
Collaborator

BoboTiG commented May 11, 2021

The test fix may still be revelant for later though.
Let's see if anyone has an idea how to deal with that then.

@BoboTiG
Copy link
Collaborator

BoboTiG commented May 12, 2021

Done with 50af6eb.

@BoboTiG BoboTiG closed this as completed May 12, 2021
@oprypin
Copy link
Contributor Author

oprypin commented May 19, 2021

I found that this delay is a general issue on Windows. There is always a chance that events right after start will be missed, just that on PyPy that chance was very high.

I am presenting 2 runs of unittests (not of watchdog but of some other project using watchdog)
where the test suite is repeated 50 times per platform

The only difference between the two runs is the added sleeps:
https://github.com/oprypin/mkdocs/compare/ac66e7de5ced56007b346ff18b7709746780970b..c6923117a39cb2a5ed335e709960997866c1300d
-- The sleeps are analogous to those that I added to watchdog.

Without sleeps, on Windows it fails 13 out of 200 times (total over all versions): https://github.com/oprypin/mkdocs/actions/runs/857713565
With sleeps passes everything: https://github.com/oprypin/mkdocs/actions/runs/857723657
(Of course, now PyPy succeeds consistently in either case :D)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants