Skip to content

Delete always skipped test, fix trio test, fix framelocal has not .clear() #1322

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.

Sign up for GitHub

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

Jump to bottom
Merged
Carreau merged 7 commits into from
Feb 19, 2025
Merged

Delete always skipped test, fix trio test, fix framelocal has not .clear() #1322

Carreau merged 7 commits into from
Feb 19, 2025

Conversation

@Carreau
Copy link
Member

@Carreau Carreau commented Feb 12, 2025 •
edited
Loading

There are few chances we will ever use them again.

Also a small refactor and unskip of trio test using trio nursery.

@Carreau Carreau force-pushed the del-always-skip branch 2 times, most recently from da04e6e to 48caeaa Compare February 12, 2025 18:38
@Carreau Carreau changed the title Delete always skipped tests that used removed attributes. Delete always skipped tests that used removed attributes, and unskip trio test Feb 12, 2025
@Carreau
Copy link
Member Author

Carreau commented Feb 12, 2025 •
edited
Loading

Well that's strange, trio test passes locally...

well on CI, anyio decides to switch in the middle of the call stack from trio to asyncio:

    |     tg.start_soon(self.kernel.start)
      |   File ".../site-packages/anyio/_backends/_trio.py", line 191, in __aexit__
      |     return await self._nursery_manager.__aexit__(exc_type, exc_val, exc_tb)  # type: ignore[return-value]
      |   File ".../site-packages/trio/_core/_run.py", line 1043, in __aexit__
      |     raise combined_error_from_nursery
      | exceptiongroup.ExceptionGroup: Exceptions from Trio nursery (1 sub-exception)
      +-+---------------- 1 ----------------
        | Traceback (most recent call last):
        |   File ".../ipykernel/ipykernel/kernelapp.py", line 753, in _wait_to_enter_eventloop
        |     await self.kernel._eventloop_set.wait()
        |   File ".../site-packages/anyio/_core/_synchronization.py", line 139, in wait
        |     await self._event.wait()
        |   File ".../site-packages/anyio/_backends/_asyncio.py", line 1764, in wait
        |     await AsyncIOBackend.checkpoint()
        |   File ".../site-packages/anyio/_backends/_asyncio.py",

@Carreau
Copy link
Member Author

Carreau commented Feb 12, 2025

Well something else is wrong on CI, some pytest items are failing, but it's shown as green on 3.10: https://github.com/ipython/ipykernel/actions/runs/13292836924/job/37117594041?pr=1322

Carreau
Carreau commented Feb 12, 2025
View reviewed changes
@Carreau Carreau mentioned this pull request Feb 12, 2025
Closed
@davidbrochart
Copy link
Collaborator

davidbrochart commented Feb 13, 2025

Well something else is wrong on CI, some pytest items are failing, but it's shown as green on 3.10: https://github.com/ipython/ipykernel/actions/runs/13292836924/job/37117594041?pr=1322

It's because we rerun last failed tests here.

@Carreau Carreau force-pushed the del-always-skip branch 3 times, most recently from acec02b to 6dd8fed Compare February 13, 2025 12:48
@Carreau Carreau changed the title Delete always skipped tests that used removed attributes, and unskip trio test Delete always skipped test, fix trio test, fix framelocal has not .clear() Feb 19, 2025
@Carreau Carreau requested a review from krassowski February 19, 2025 07:15
@Carreau
Copy link
Member Author

Carreau commented Feb 19, 2025 •
edited
Loading

fix framelocal has not .clear()

It would be really great to get that merged, as this fixes failures of other PRs

davidbrochart
davidbrochart reviewed Feb 19, 2025
View reviewed changes
@Carreau
Copy link
Member Author

Carreau commented Feb 19, 2025

(docs breckage is due to new sphinx release and unrelated to us)

davidbrochart
davidbrochart reviewed Feb 19, 2025
View reviewed changes
krassowski
krassowski approved these changes Feb 19, 2025
View reviewed changes
Copy link
Member

@krassowski krassowski left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good to me. The sphinx issue is tracked in tox-dev/sphinx-autodoc-typehints#523

@krassowski krassowski mentioned this pull request Feb 19, 2025
Merged
@Carreau
Copy link
Member Author

Carreau commented Feb 19, 2025

Thanks for the reviews. Merging.

@Carreau Carreau merged commit 28d096d into ipython:main Feb 19, 2025
26 of 30 checks passed
@Carreau Carreau added this to the 7.0 milestone Feb 23, 2025
ianthomas23 added a commit to ianthomas23/ipykernel that referenced this pull request Apr 11, 2025
@ianthomas23
Fix embed kernel (taken from ipython#1322)
05ee4fd
@ianthomas23 ianthomas23 mentioned this pull request Apr 11, 2025
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@davidbrochart davidbrochart davidbrochart left review comments

@minrk minrk Awaiting requested review from minrk

+1 more reviewer

@krassowski krassowski krassowski approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

maintenance

Projects

None yet

Milestone

7.0

Development

Successfully merging this pull request may close these issues.

3 participants

@Carreau @davidbrochart @krassowski