openedx / openedx/frontend-app-learning

Make the courseware redirect-rule tests actually assert: un-awaited `waitFor` and unwired mocks from #1501

Open
#2,078 0 comments 1 reaction 1 assignee View on GitHub

@brian-smith-tcril is already working on this.

Since Sep 19, 2026.

Dominant language
JavaScript
Stars
70
Forks
335
Avg merge
1d 17h
Merged PRs (30d)
35

Description

Part of #1946 — Redux → React Query migration (Stage 1). Not a conversion: a test-hygiene fix on code the migration moved, filed here because it sits in the courseware redirect rules that the remaining layers build on and should be cleaned up before those continue.

What's wrong. The redirect-rule unit tests in src/courseware/redirects.test.ts have 13 waitFor(...) calls that are never awaited, inside synchronous it(...) callbacks. An un-awaited waitFor returns a promise nobody observes, so its callback's failure surfaces after the test has already passed and Jest drops or misattributes it. Every assertion inside those 13 blocks is therefore vacuous today. They cover the two async rules:

  • unitToSequenceUnitRedirect — 4 tests in the isPreview equals true block, 3 in isPreview equals false
  • resumeRedirect — 4 tests in isPreview equals true, 2 in isPreview equals false

Most of them also assert on mocks that are never wired to anything: const getSequenceForUnitDeprecated = jest.fn(); (6 tests) and const getResumeBlock = jest.fn(); (2 tests) are declared locally, never passed to or used by the rule under test, and then asserted with toHaveBeenCalled(). Those expectations can never pass, which is why simply adding await would turn vacuous tests into failing ones. The real behavior under test happens through axiosMock handlers on the resume and block-children endpoints, which the tests do set up.

Because the navigate(...) expectations have never been exercised either, some are likely wrong. Reading the rule: the isPreview equals falseresumeRedirect › "calls navigate with unitId" case passes isPreview: false but expects /preview/course/courseId/section_1/unit_1; resumeRedirect only prefixes /preview when isPreview is true. Confirm by running once the awaits are in.

Provenance. All of this arrived together in #1501 (2024-10-28, "add functionality to see unit draft preview"), which added the redirect-rule tests to CoursewareContainer.test.jsx. It predates the migration by two years. #2074 moved the block into redirects.test.ts alongside the extracted rules and converted the call signatures to options objects, but carried the un-awaited calls and unwired mocks across unchanged; master's CoursewareContainer.test.jsx has the same 13 until #2074 lands. Flagged by @arbrandes on #2074 (comment) as non-blocking; deliberately left out of that PR so the layer stays a faithful move and the cleanup can be reviewed on its own.

Precedent. 65dbeb86 (PR #2060) did the same for four fire-and-forget waitFor calls in Course.test.jsx: await the ones with real assertions, and rewrite the ones whose matcher was vacuous into assertions that genuinely check something.

Tasks

  • Make the 13 tests async and await their waitFor calls.
  • Replace the unwired getSequenceForUnitDeprecated / getResumeBlock mock assertions with assertions on what actually happens: the request to the mocked endpoint (axiosMock.history.get) and the resulting navigate call (or its absence).
  • Run the file and correct any navigate URL expectation that turns out never to have matched the rule; note each one in the PR.
  • Keep the change test-only; redirects.ts is not touched.

Verify: redirects.test.ts green with every waitFor awaited (grep -c "await waitFor" equals grep -c "waitFor("), no jest.fn() in the file that isn't passed to the code under test, and each async-rule test fails if its navigate expectation is deliberately broken.

[!NOTE]
This issue was written by Claude (Claude Code) and reviewed before posting.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.