Skip to content

Add delete todos test suite - #57

Open
DenysKuchma wants to merge 2 commits into
masterfrom
testeiya-demo-review
Open

DenysKuchma wants to merge 2 commits into
masterfrom
testeiya-demo-review

Conversation

@DenysKuchma

Copy link
Copy Markdown
Contributor

No description provided.

@DenysKuchma
DenysKuchma requested a review from DavertMik November 2, 2026 19:18
@github-actions

github-actions Bot commented Nov 2, 2026

Copy link
Copy Markdown

📋 Requirements

PR requirements: delete todos suite and selector fix

Scope read: branch testeiya-demo-review vs master (no PR number in the run; assumed this branch). Commits 59cc94c and 3ee2d47, plus the workflow fix in ed6b23f that is on master but not on this branch. The /qa-pr-requirements-analyzer skill is not installed in this environment, so this analysis is done manually from the diff.

What the PR is meant to do
  • Add automated UI tests for deleting todos on the TodoMVC React app.
  • Requirements implied by the tests:
    1. Deleting the only todo leaves an empty list.
    2. Deleting one todo from several removes only that todo and keeps the order of the rest.
    3. Deleting todos one by one empties the list and the "items left" footer disappears.
  • Fix the broken selector label[for="toggle-all" in markAllAsCompleted (missing ]).
  • Update the suite README with the new case.
  • Add a manual-dispatch Testeiya review workflow (on master).
Findings

High

  • The new suite is not tagged @full-browser, but deleting relies on hover (moveCursorTo) to reveal the delete button. The repo's own README says hover-revealed controls do not work on alternative browsers. The Obscura, Lightpanda and Moli configs only exclude @full-browser, so these three scenarios will run there and are likely to fail. The existing delete test in edit-todos_test.js is tagged for this reason.

Medium

  • Coverage overlaps with edit-todos_test.js ("Delete todos @full-browser"). Two suites now cover deletion with different tags, and it is unclear which one is the source of truth.
  • delete-todos.steps.js (Gherkin steps) is dead code. No feature file uses these steps, and codecept.conf.js loads only create-todos.steps.js.
  • The selector fix has no test. markAllAsCompleted is only used by a skipped scenario (Scenario.skip('Mark all todos as completed')), so the fix is unverified.

Low

  • Feature tag @step:11 does not match the existing @step:06 / @step-06 convention.
  • The Before hook calls TodosPage.goto() without await; this matches the other suites but is fragile.
  • Scenario 3 is titled "until the list is empty" but only checks the empty state after two deletes of the first item; it does not test deleting the last remaining item from a longer list.
  • ed6b23f (the review-comment fix) is on master but not on this branch. Rebase before the PR-review workflow runs against it.
Blockers
  • None for the analysis. The PR number and the skill file were not available, so the branch was inferred.
  • No Testomat.io link: requirements could not be checked against any test-management tracker.

🔍 Review

Failed: New delete scenarios lack @full-browser tag and will fail on the Obscura, Lightpanda and Moli configs (hover-dependent)

QA review: delete todos suite and selector fix

Verdict: fail. The delete tests break the alternative-browser configs the repo ships.

Requirements coverage
Requirement Covered by Status
Deleting the only todo leaves an empty list Delete a single todo item Covered (static review)
Deleting one of several keeps the others in order Delete one of multiple todos keeps the others in order Covered (static review)
Deleting one by one empties the list and hides the footer Delete todos one by one until the list is empty Covered (static review)
Fix label[for="toggle-all" selector Clear completed todos (active, mark-as-completed_test.js) Covered indirectly
README updated todomvc-tests/README.md Done

Browser tests were not run (no browser allowed in this run). Coverage is judged from the code.

Findings

High

  • The three new delete scenarios are not tagged @full-browser, but deleteNthTodo depends on hover to reveal the delete button. The Obscura, Lightpanda and Moli configs exclude only @full-browser, so these scenarios run there and will fail. The repo's own edit-todos_test.js tags its delete test for this exact reason.
  • Impact: npm run test:obscura, test:lightpanda and test:moli go red on this PR. The default Playwright CI job (test.yml) is not affected.
Corrections to requirements.md
  • The selector fix is not "unverified". markAllAsCompleted is used by the active scenario Clear completed todos. The Mark all todos as completed scenario is skipped, but the other use is not. The broken selector sat in an active test on master, so the fix matters for the default run.
  • The @step tag remark is about tag style only; both forms already exist in the repo.
Required before merge
  1. Tag the three new delete scenarios @full-browser, or otherwise exclude them from the alternative-browser configs.
  2. Resolve the overlap with edit-todos_test.js: keep one delete suite as the source of truth.
  3. Remove delete-todos.steps.js or wire it into a config and feature.
  4. Rebase onto master to pick up ed6b23f.

📝 Test cases

Skipped: No test cases created. Enable test case creation via CREATE_TESTCASES env var

🔄 Sync to Testomat.io

Skipped: No Testomat.io project. Set TESTOMATIO.

🧚🏻‍♀️ Provided by Testeiya QA Agent & claude-haiku-5-5

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