MTV-3643 | E2E customization scripts - downstream spec & page objects (part 2)#2341
Conversation
Pedro-S-Abreu
force-pushed
the
e2e-customization-scripts
branch
from
6ff0e76 to
6207805
Compare
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #2341 +/- ##
===========================================
- Coverage 36.81% 16.27% -20.55%
===========================================
Files 158 1192 +1034
Lines 2548 21664 +19116
Branches 599 4284 +3685
===========================================
+ Hits 938 3525 +2587
- Misses 1428 18129 +16701
+ Partials 182 10 -172 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
avivtur
left a comment
avivtur
left a comment
There was a problem hiding this comment.
nice work, left some comments
| for (let i = existingCount - 1; i > 0; i -= 1) { | ||
| await this.scriptEditModal.removeScript(i); | ||
| } | ||
|
|
||
| for (let i = 0; i < scripts.length; i += 1) { | ||
| if (i > 0) { | ||
| await this.scriptEditModal.addScript(); | ||
| } | ||
| await this.scriptEditModal.configureScript(i, scripts[i]); | ||
| } |
There was a problem hiding this comment.
why you remove and than add, and something looks weird in 2nd for loop, why using that if? why not configure and than add?
There was a problem hiding this comment.
added comment
There was a problem hiding this comment.
I meant the logic here is a bit off, why we need the remove and add operations, can't you just configure directly?
| const LINUX_FIRSTBOOT_SCRIPT: ScriptConfig = { | ||
| content: '#!/bin/bash\necho "firstboot setup"\nsystemctl enable my-service', | ||
| guestType: 'linux', | ||
| name: 'linux-setup', | ||
| scriptType: 'firstboot', | ||
| }; | ||
|
|
||
| const WINDOWS_FIRSTBOOT_SCRIPT: ScriptConfig = { | ||
| content: 'REM Windows firstboot\nnet user admin P@ssw0rd /add', | ||
| guestType: 'windows', | ||
| name: 'win-setup', | ||
| scriptType: 'firstboot', | ||
| }; | ||
|
|
||
| const UPDATED_SCRIPT: ScriptConfig = { | ||
| content: '#!/bin/bash\necho "updated script"\nexit 0', | ||
| guestType: 'linux', | ||
| name: 'updated-script', | ||
| scriptType: 'run', | ||
| }; |
There was a problem hiding this comment.
can we use SCRIPT_TYPE_LABELS and GUEST_TYPE_LABELS here with toLowerCase?
There was a problem hiding this comment.
GUEST_TYPE_LABELS maps from 'linux' to 'Linux', so doing GUEST_TYPE_LABELS['linux'].toLowerCase() to get 'linux' back is circular.
| this.scriptEditModal = new ScriptEditModal(page); | ||
| } | ||
|
|
||
| async editScripts(scripts: ScriptConfig[]): Promise<void> { |
There was a problem hiding this comment.
I think the name of the function is what got me confused about the logic here in the first place, is this function essentially replacing the existing scripts with the scripts coming as parameter?
if so I think you'd want some function like setScripts(scripts: ScriptConfig[]): Promise in ScriptEditModal page object that would look like your original implementation before the latest change, then here you'd have replaceScripts that could like this:
async replaceScripts(scripts: ScriptConfig[]): Promise {
await this.openScriptEditModal();
await this.scriptEditModal.setScripts(scripts);
await this.scriptEditModal.save();
}
this would clarify intent of open-> set new scripts-> save
… (part 2) Add downstream Playwright E2E tests for the customization scripts feature, using the page objects introduced in part 1. Resolves: MTV-3643 Signed-off-by: Pedro Abreu <pabreu@redhat.com>
Move GUEST_TYPE_LABELS and SCRIPT_TYPE_LABELS to test-data.ts and import from both page objects that used local copies. Resolves: MTV-3643 Signed-off-by: Pedro Abreu <pabreu@redhat.com>
Resolves: MTV-3643 Signed-off-by: Pedro Abreu <pabreu@redhat.com>
- Remove unused verifyScriptName and verifyEditButtonDisabled
- Replace waitForLoadState('networkidle') with click-only
- Fix misleading step name
Resolves: MTV-3643
Signed-off-by: Pedro Abreu <pabreu@redhat.com>
- Replace hardcoded 'Linux', 'Windows', 'Firstboot', 'Run' strings with GUEST_TYPE_LABELS and SCRIPT_TYPE_LABELS lookups in spec assertions - Add clarifying comment on editScripts explaining the row-reuse pattern Resolves: MTV-3643 Signed-off-by: Pedro Abreu <pabreu@redhat.com>
Move configureScript logic (field filling, dropdown selection, Monaco editor content) into a shared fillScriptFields utility. Both ScriptEditModal and CustomizationScriptsStep now delegate to it, parameterized by their different test IDs. Resolves: MTV-3643 Signed-off-by: Pedro Abreu <pabreu@redhat.com>
Instead of removing all rows then adding back, reuse existing rows, only add when more are needed, and remove only the excess. Resolves: MTV-3643 Signed-off-by: Pedro Abreu <pabreu@redhat.com>
Move row management (configure/add/remove) into ScriptEditModal.setScripts() and rename the AutomationTab orchestrator to replaceScripts() for clearer intent: open modal -> set scripts -> save. Resolves: MTV-3643 Signed-off-by: Pedro Abreu <pabreu@redhat.com>
avivtur
force-pushed
the
e2e-customization-scripts
branch
from
9c2118b to
4113e4d
Compare
|
3 participants
📝 Links
📝 Description
Adds downstream Playwright E2E tests for the customization scripts feature, complementing the page objects introduced in part 1.
New files:
plan-customization-scripts.spec.ts— 3 downstream tests (create with scripts, empty state, back-navigation edit)AutomationTab.ts— Plan Details Automation tab page objectScriptEditModal.ts— Script edit modal page objectModified:
PlanDetailsPage.ts— wiredAutomationTabCustomizationScriptsStep.ts— fixed heading text to match actual wizard step title🎥 Demo