Categorising playwright tests into folders - Eagle 1684 - #1075
Merged
Merged
Conversation
M-Wicenec
requested review from
james-strauss-uwa
and
a lite review from Copilot
September 22, 2026 08:37
Contributor
Reviewer's GuideThe PR categorizes Playwright end-to-end tests into graph, palette, repository, tutorial, and undo folders, updates imports and test-step structure, adds focused undo/tutorial coverage, removes obsolete root-level copies, and cleans up tutorial logging. File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Hey - I've found 2 issues
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="e2e/undo/undoAfterFixAll.spec.ts" line_range="37" />
<code_context>
+ await page.locator('div[data-notify="container"]').waitFor({ state: 'attached' });
+ await page.locator('button[data-notify="dismiss"]').click();
+ await page.locator('div[data-notify="container"]').waitFor({ state: 'detached' });
+ await expect.poll(async () => await TestHelpers.getNumWarningsErrors(page)).toBeLessThanOrEqual(initialCount);
+ postFixCount = await TestHelpers.getNumWarningsErrors(page);
+ console.log('Post-fix warnings+errors:', postFixCount);
+ });
</code_context>
<issue_to_address>
**issue (testing):** The assertion only requires the post-fix issue count to be less than or equal to the initial count, so the test passes when pressing `f` does nothing and no issues are fixed. The later undo assertion then verifies a state that was never confirmed to be produced by `fixAll`.
**Triggers:** When the fix-all keyboard shortcut is broken or does not apply any fixes.
**Suggested fix:** Assert that the fix operation changes the issue state, for example by requiring `postFixCount` to be strictly less than `initialCount` when this fixture is expected to contain fixable issues.
```suggestion
await expect.poll(async () => await TestHelpers.getNumWarningsErrors(page)).toBeLessThan(initialCount);
```
</issue_to_address>
### Comment 2
<location path="e2e/graph/validatingGraphs.spec.ts" line_range="18-22" />
<code_context>
+test('Validating Graphs', async ({ page }) => {
+ for (const graphUrl of GRAPHS){
+ await test.step(`Validate graph: ${graphUrl}`, async () => {
+ await page.goto('http://localhost:8888/?tutorial=none&service=Url&url='+graphUrl);
+ await TestHelpers.waitForNotificationAndDismiss(page);
+ await TestHelpers.openGraphMenuAndSelect(page, 'validateGraph');
+ await page.locator('div[data-notify="container"]').waitFor({state: 'attached'});
+ await expect(page.locator('span[data-notify="message"]')).toContainText(" valid ");
+ });
+ }
</code_context>
<issue_to_address>
**issue (testing):** The validation notification is never dismissed before the loop proceeds to the next graph. On the next iteration, `waitForNotificationAndDismiss` can dismiss the previous graph's validation result instead of waiting for the new graph-load notification, allowing validation to run against stale or incompletely loaded graph state.
**Triggers:** When the validation notification remains attached while the next graph is loaded.
**Suggested fix:** Dismiss and wait for detachment of the validation result at the end of each iteration, and explicitly wait for the next graph-load notification before opening the validation menu.
</issue_to_address>Sourcery assessment
Approval pending. 2 findings to address first.
Blocking findings: e2e/undo/undoAfterFixAll.spec.ts:37, e2e/graph/validatingGraphs.spec.ts:22
There was a problem hiding this comment.
Copilot review overview
🔵 Needs a closer look
Three relocated graph specs still resolve a fixture from the old directory and fail before page load.
Review effort: Lite
Findings: None
What changed in this PR
Reorganizes the Playwright E2E suite by domain and expands workflow and regression coverage.
Changes:
- Groups graph, palette, repository, tutorial, and undo tests.
- Updates helper imports and adds structured test steps.
- Removes obsolete root-level specs and tutorial logging.
| File | Summary |
|---|---|
e2e/validatingGraphs.spec.ts |
Removed after relocation. |
e2e/v4FormatJsonMatch.spec.ts |
Removed after relocation. |
e2e/undoDuplicateDetection.spec.ts |
Removed after relocation. |
e2e/undoBoundary.spec.ts |
Removed after relocation. |
e2e/undoAfterFixAll.spec.ts |
Removed after relocation. |
e2e/undo/undoRedo.spec.ts |
Updated imports and test steps. |
e2e/undo/undoDuplicateDetection.spec.ts |
Added categorized undo regression coverage. |
e2e/undo/undoBoundary.spec.ts |
Added undo-boundary coverage. |
e2e/undo/undoAfterFixAll.spec.ts |
Added fix-and-undo regression coverage. |
e2e/tutorials/tutorials.spec.ts |
Added categorized tutorial coverage. |
e2e/tutorials.spec.ts |
Removed after relocation. |
e2e/TestHelpers.ts |
Removed tutorial-start logging. |
e2e/repositories/addingAndRemovingRepositories.spec.ts |
Updated helper import. |
e2e/parameterTableAndKeyboardShortcuts.spec.ts |
Removed after relocation. |
e2e/palettes/creatingAndEditingPalettes.spec.ts |
Updated imports and step structure. |
e2e/palettes/addGraphNodesToPalette.spec.ts |
Added categorized palette coverage. |
e2e/loadSaveJsonMatch.spec.ts |
Removed after relocation. |
e2e/insertGraph.spec.ts |
Removed after relocation. |
e2e/graph/validatingGraphs.spec.ts |
Added categorized graph validation coverage. |
e2e/graph/v4FormatJsonMatch.spec.ts |
Added format round-trip coverage; fixture path needs correction. |
e2e/graph/parameterTableAndKeyboardShortcuts.spec.ts |
Added categorized parameter coverage. |
e2e/graph/loadSaveJsonMatch.spec.ts |
Added JSON round-trip coverage; fixture path needs correction. |
e2e/graph/insertGraph.spec.ts |
Added graph insertion coverage; fixture path needs correction. |
e2e/graph/findEdgesContainedByNodes.spec.ts |
Updated helper import. |
e2e/graph/creatingASimpleGraph.spec.ts |
Added categorized graph creation coverage. |
e2e/graph/cloneLogicalGraph.spec.ts |
Updated helper import. |
e2e/example.spec.ts |
Removed obsolete sample tests. |
e2e/creatingASimpleGraph.spec.ts |
Removed after relocation. |
e2e/addGraphNodesToPalette.spec.ts |
Removed after relocation. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
M-Wicenec
changed the title
…the same number as before
…ation before doing the next one
Collaborator
Author
|
@james-strauss-uwa ready for your review. |
Collaborator
|
@copilot review |
james-strauss-uwa
approved these changes
Sep 30, 2026
james-strauss-uwa
left a comment
james-strauss-uwa
left a comment
Collaborator
There was a problem hiding this comment.
Nice improvement
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
categorising tests into folders
Summary by Sourcery
Organize and extend the Playwright end-to-end test suite by grouping scenarios by functionality and adding coverage for graph, tutorial, palette, and undo workflows.
Enhancements:
Tests:
Chores: