-
-
Notifications
You must be signed in to change notification settings - Fork 3k
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
[WIP] MultitouchSimulatorTestCase: Fix CI failing test #7476
[WIP] MultitouchSimulatorTestCase: Fix CI failing test #7476
Conversation
Funnily enough, I am able to reproduce in this in a VirtualBox VM running Mint 20.1. If it would help, I could breakpoint and get you debugging info, if that would help?
|
As it turns out, the kivy/kivy/tests/test_mouse_multitouchsim.py Line 261 in 53b655b
|
I assumed it is there to test that the key exists, rather to use the color in some way. |
@pythonic64 So, I've pushed changes (accidentally. I'm an idiot. What can I say. But we previously had branch protection, which we need to re-instate. Will follow up...). These changes remove redundant value retrievals which directly affect this issue. May I suggest you merge these changes from master, and report back? Do you think there is a real issue here, or were they just invalid tests? Thanks |
Well, that's the question. Is the failing test on Windows a real issue, or is it just an artifact. To answer I think we'd need to understand what is being accessed and why it's failing (only on VM). Since we can't easily do it on the CI, it's good to know it can be replicated on VM because that'll make it easier to isolate the issue, |
I'm going to suggest we just leave that removed. It effectively tests a hidden property, so is already of dubious value. Tests should ideally be limited to very specific things. @matham Agreed? To exploit this opportunity, please could you quickly check this: #7466 |
I agree we should leave it removed for now, because it's important for the tests to work and not show unrelated errors in submitted PRs. However, it would be nice if eventually someone works out why it failed because it makes me uncomfortable to have random failures without knowing why. |
@dotrichard Removing unused I run the tests on VM Ubuntu 18 and it did not reproduce the failing test. Can you collect values from |
Since removing that |
@Zen-CODE I don't think so, it just hides the problem. Both
|
I think you'd probably only be able to reproduce on Windows VM (maybe only on mint). Because it originally doesn't happen on Ubuntu CI. |
@matham Yes, but I remembered that I have virtual Ubuntu 18, so I gave it a try to see if test will fail. But they didn't :) |
Still failing, but now with referencing ellipse fails.
I enabled the test with the latest master in this pull request: #7483. |
@pythonic64 Can't reproduce it either. As you mentioned, I think you're going to need to log that state and capture it somehow, as it's not clear how to get those from the tests? And does it only occur on Window x86? I see I still have a 32-bit Win7 ISO that I might be able to test on, but that's a last resort.... |
This error doesn't occur on specific Python/Windows version, it's almost "random" as tests on some Python/Windows combination will pass and then on one will fail. |
f25535f
to
7a0352c
Compare
+1 for the effort. It is appreciated. |
9ac2801
to
0ff3689
Compare
4a7e10b
to
7956726
Compare
7956726
to
35e5bb6
Compare
…t render calls in MultitouchSimulatorTestCase.
Possible fix for
MultitouchSimulatorTestCase
which fails ifMouseHoverEventTestCase
is enabled.Maintainer merge checklist:
Component: xxx
label.api-deprecation
orapi-break
label.release-highlight
label to be highlighted in release notes.versionadded
,versionchanged
as needed.