Skip to content

chore: optimize critical flow tests runner [WPB-17563]#20866

Open
e-maad wants to merge 8 commits intodevfrom
chore/optimize-crit-flow-WPB-17563
Open

chore: optimize critical flow tests runner [WPB-17563]#20866
e-maad wants to merge 8 commits intodevfrom
chore/optimize-crit-flow-WPB-17563

Conversation

@e-maad
Copy link
Copy Markdown
Contributor

@e-maad e-maad commented Mar 26, 2026

TaskWPB-17563 [Web] - Add critical flow tests to precommit pipeline

Summary

Optimize critical flow tests runner.

Security Checklist (required)

  • External inputs are validated & sanitized on client and/or server where applicable.
  • API responses are validated; unexpected shapes are handled safely (fallbacks or errors).
  • No unsafe HTML is rendered; if unavoidable, sanitization is applied and documented where it happens.
  • Injection risks (XSS/SQL/command) are prevented via safe APIs and/or escaping.

Accessibility (required)

Standards Acknowledgement (required)


Screenshots or demo (if the user interface changed)

Notes for reviewers

  • Trade-offs:
  • Follow-ups (linked issues):
  • Linked PRs (e.g. web-packages):

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Mar 26, 2026

🔗 Download Full Report Artifact

🧪 Playwright Test Summary

  • Passed: 8
  • Failed: 0
  • Skipped: 0
  • 🔁 Flaky: 6
  • 📊 Total: 14
  • Total Runtime: 182.4s (~ 3 min 2 sec)
specs/CriticalFlow/accountManagement-TC-8639.spec.ts (❌ 0 failed, ⚠️ 1 flaky)
  • ⚠️ Account Management (tags: TC-8639, crit-flow-web)
specs/CriticalFlow/backupRestoration-TC-8634.spec.ts (❌ 0 failed, ⚠️ 1 flaky)
  • ⚠️ Setting up new device with a backup (tags: TC-8634, crit-flow-web)
specs/CriticalFlow/groupCalls-TC-8632.spec.ts (❌ 0 failed, ⚠️ 1 flaky)
  • ⚠️ Planning group call with sending various messages during call (tags: TC-8632, crit-flow-web)
specs/CriticalFlow/messagesInChannels-TC-8753.spec.ts (❌ 0 failed, ⚠️ 1 flaky)
  • ⚠️ Messages in Channels (tags: TC-8753, crit-flow-web)
specs/CriticalFlow/messagesInGroups-TC-8751.spec.ts (❌ 0 failed, ⚠️ 1 flaky)
  • ⚠️ Messages in Groups (tags: TC-8751, crit-flow-web)
specs/CriticalFlow/personalAccountLifecycle-TC-8638.spec.ts (❌ 0 failed, ⚠️ 1 flaky)
  • ⚠️ Personal Account Lifecycle (tags: TC-8638, crit-flow-web)

@sonarqubecloud
Copy link
Copy Markdown

ls -la $HOME/.local/bin/op
echo "=== All paths verified ==="

- name: Cache dependencies for test shards
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not convinced the dependency caching is a good tradeoff here. The 64 Playwright shards do make repeated setup expensive, so I agree with the goal of avoiding 64 separate dependency installs. My concern is with the mechanism, not the optimization target.

Because the cache key includes github.run_id, this is not really a reusable cache across workflow runs. We save a large dependency tree once, restore it many times within the same run, and then delete it again. That makes actions/cache behave more like an intra-run artifact transport than a true cache.

That adds complexity and fragility around node_modules, Playwright binaries, and the 1Password CLI. The follow-up fixes below in this branch makes that brittleness already showing up.

So I'm not arguing against reducing repeated setup for 64 shards. I'm arguing that this implementation may be the wrong tradeoff.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants