Skip to content

Migrate rest_tornado saltnado tests to pytest#69704

Open
ggiesen wants to merge 3 commits into
saltstack:3006.xfrom
ggiesen:migrate-saltnado-tests-pytest
Open

Migrate rest_tornado saltnado tests to pytest#69704
ggiesen wants to merge 3 commits into
saltstack:3006.xfrom
ggiesen:migrate-saltnado-tests-pytest

Conversation

@ggiesen

@ggiesen ggiesen commented Jul 4, 2026

Copy link
Copy Markdown
Contributor

What does this PR do?

Migrates the rest_tornado saltnado test suite from the legacy tests/unit
tree to pytest. The 27 tests across the three AsyncTestCase classes
(TestJobNotRunning, TestGetMinionReturns, TestDisbatchLocal) are ported
to plain pytest functions under tests/pytests/unit/netapi/rest_tornado/,
driving the handler coroutines with the existing io_loop fixture
(io_loop.run_sync) instead of tornado.testing.gen_test, and the legacy
file is removed.

Every test is preserved one-for-one with the same mocks, opts, event
sequences and expected values; assertions are converted at equal-or-stronger
strength (assertIs -> is, assertDictEqual -> ==, etc.). No behaviour
of salt/netapi/rest_tornado/saltnado.py is changed -- this is tests only.

What issues does this PR fix or reference?

Part of the ongoing migration of the legacy tests/unit suite to pytest.

Previous Behavior

The saltnado handler coroutine tests lived in
tests/unit/netapi/rest_tornado/test_saltnado.py as tornado.testing.AsyncTestCase
subclasses.

New Behavior

The same 27 tests run as pytest functions in
tests/pytests/unit/netapi/rest_tornado/test_saltnado.py. Verified locally:
27 passed, and each legacy test was confirmed to have an equal-or-stronger
pytest counterpart before removing the legacy file.

Merge requirements satisfied?

  • Tests written/updated (migrated)
  • Changelog (not applicable -- test-only move)

Commits signed with GPG?

No

@ggiesen ggiesen requested a review from a team as a code owner July 4, 2026 21:45
Port the AsyncTestCase suite in
tests/unit/netapi/rest_tornado/test_saltnado.py to pytest, driving the
handler coroutines with the io_loop fixture, and remove the legacy file.
- Bound every io_loop.run_sync call with a 30 second timeout. The
  legacy gen_test decorator enforced a per-test deadline; run_sync has
  none by default and the harness fallback timeout is not applied on
  Windows, so a hang regression (the exact case the infinite-loop guard
  test exists for) would have consumed the whole job.
- Re-raise exceptions from IOLoop callbacks at test teardown via an
  io_loop fixture wrapper. AsyncTestCase failed a test when a scheduled
  callback raised; the plain loop only logs, which would let a broken
  completer pass a test that no longer exercises anything.
- Share one handler fixture (conftest.py) instead of three copies of
  the same mock scaffold, resolving the 2020 TODO carried over from the
  unittest file, and move test_base_handler.py in from the sibling
  saltnado directory so the module's unit tests live in one place and
  consume the same fixture.
- Make the two timer-based _disbatch_local tests deterministic by
  routing the gather timeout through an already-resolved fake sleep,
  removing a real 2 second wait and a 50 millisecond ordering race.
- Drop the dead boop() generators (never iterated; spawn_callback of a
  bare generator is a no-op), differentiate the two byte-identical
  disbatch-after-finish tests by asserting the response buffer is not
  written to, and remove the now-empty legacy tests/unit/netapi
  packages.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant