-
-
Notifications
You must be signed in to change notification settings - Fork 396
Disable 3 failing downstream tests, but keep testing the rest. #1349
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
Conversation
ab34145 to
cb7d278
Compare
7beacf2 to
d899ce6
Compare
|
Just a quick comment to say that I don't agree with this change because it'd basically leave this in the cold to support IPykernel 7.0, which seems to include some big changes (like the usage of AnyIO) for which we have zero experience with. Furthermore, last time I checked there are not many failing tests in both Qtconsole and Spyder-kernels that need to be fixed. |
The public API is just async, you don't need experience with AnyIO. |
|
This disable just the failing tests that have been failing for months:
Not the full test suite Currently every PRs here need an admin to overwrite merge because of only these 3 failures, So it does not change problem, it will just make it easier to spot if there are any other regressions. |
|
In addition, if you look, I removed the |
We have little experience with async stuff because it's not easy to integrate it with Qt and it doesn't bring a significant advantage for us due to the GIL.
Ok, sorry for the noise then. I thought you were going to disable the entire test suite.
Ok, at least in spyder-kernels we have an upper constraint on IPykernel 7, so we can take our time to address those failures. But that's not the case in Qtconsole, and the test marked as failing checks the in-process kernel, which is used by several projects downstream. So hopefully that can be addressed before releasing the new version. |
|
No worries, I've rephrased the PR title. Let me merge this and open an issue marked 7.0 to remind us to un-skip those tests. |
Also remove the -x, because if there is many failures, we want to knw that.