Skip to content

Conversation

@Carreau
Copy link
Member

@Carreau Carreau commented Feb 19, 2025

Also remove the -x, because if there is many failures, we want to knw that.

@Carreau Carreau force-pushed the disable-downstream branch 3 times, most recently from ab34145 to cb7d278 Compare February 19, 2025 15:15
@ccordoba12
Copy link
Member

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.

@davidbrochart
Copy link
Collaborator

which seems to include some big changes (like the usage of AnyIO) for which we have zero experience with

The public API is just async, you don't need experience with AnyIO.

@Carreau
Copy link
Member Author

Carreau commented Feb 19, 2025

This disable just the failing tests that have been failing for months:

  • only 2 tests in spyder_kernel
  • and 1 test in qtconsole

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.

@Carreau
Copy link
Member Author

Carreau commented Feb 19, 2025

In addition, if you look, I removed the -x, which meant the test were stopping after the first failure, so if anything this PRs tests qtconsole and spyder_kernel more than current main

@ccordoba12
Copy link
Member

The public API is just async, you don't need experience with AnyIO.

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.

This disable just the failing tests that have been failing for months

Ok, sorry for the noise then. I thought you were going to disable the entire test suite.

only 2 tests in spyder_kernel
and 1 test in qtconsole

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.

@Carreau Carreau changed the title Disable failing downstream tests Disable 3 failing downstream tests, but keep testing the rest. Feb 20, 2025
@Carreau
Copy link
Member Author

Carreau commented Feb 20, 2025

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.

@Carreau Carreau merged commit f1609ea into ipython:main Feb 20, 2025
32 of 33 checks passed
@Carreau Carreau added this to the 7.0 milestone Feb 20, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants