#9156: tab synchronized mod variable storage (3/3) #9181

twschiller · 2024-09-20T22:56:48Z

What does this PR do?

Closes Slice 3: Add state controller support for tab synchronization policy #9156
Stacked on #9154: mod variable session sync (2/3) #9155
Cleans up browser.storage.session on tab closure and mod deactivation
Refactoring: splits up stateController

Remaining Work

Rebase vs. main

Demo

See Playwright test

Future Work

Playwright fixtures to assert browser.session.storage contents, to automatically check that the tab/deactivation listeners are working properly

For more information on our expectations for the PR process, see the
code review principles doc

…-migration

src/platform/state/sessionStateHelpers.ts

…-migration

grahamlangford · 2024-09-23T14:06:37Z

src/background/stateControllerListeners.test.ts

+    let values = await browser.storage.session.get();
+    expect(Object.keys(values)).toHaveLength(2);
+
+    // Delete unrelated mod variables
+    await deleteSynchronizedModVariablesForMod(modComponentRefFactory().modId);
+    values = await browser.storage.session.get();
+    expect(Object.keys(values)).toHaveLength(2);
+
+    // Delete mod variables
+    await deleteSynchronizedModVariablesForMod(modComponentRef.modId);
+    values = await browser.storage.session.get();
+    expect(Object.keys(values)).toHaveLength(0);


NIT: This test could be improved by having sessionStorage have variables for 2 different mod ids and confirming that only the variables for the intended modId are deleted

I think that's covered here? https://github.com/pixiebrix/pixiebrix-extension/pull/9181/files#diff-d96109bff41bb64c110368123ab2d2bbbc6cf307123d1dbbb9f4d4ca9919398cR73

src/contentScript/stateController/stateController.test.ts

github-actions · 2024-09-23T15:23:45Z

No loom links were found in the first post. Please add one there if you'd like to it to appear on Slack.

Do not edit this comment manually.

Add state controller test 9154: add E2E runtime test Improve comments Refactor dipatch method Fix type errors

Improve method names Add jest tests Split up state controller Remove performance note

codecov · 2024-09-23T22:44:17Z

Codecov Report

Attention: Patch coverage is 88.11881% with 12 lines in your changes missing coverage. Please review.

Project coverage is 74.81%. Comparing base (8318d74) to head (86d0070).
Report is 312 commits behind head on main.

Files with missing lines	Patch %	Lines
...ipt/stateController/modVariablePolicyController.ts	80.00%	8 Missing ⚠️
src/background/stateControllerListeners.ts	88.23%	2 Missing ⚠️
src/background/background.ts	0.00%	1 Missing ⚠️
src/store/deactivateUtils.ts	0.00%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #9181      +/-   ##
==========================================
+ Coverage   74.24%   74.81%   +0.56%     
==========================================
  Files        1332     1364      +32     
  Lines       40817    42036    +1219     
  Branches     7634     7850     +216     
==========================================
+ Hits        30306    31448    +1142     
- Misses      10511    10588      +77

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

github-actions · 2024-09-23T23:02:30Z

Playwright test results

132 passed
2 flaky
4 skipped

Details

Open report ↗︎
138 tests across 45 suites
25 minutes, 57 seconds
86d0070
For more information on how to debug and view this report, see our readme

Flaky tests

chrome › tests/pageEditor/addStarterBrick.spec.ts › Add starter brick to mod
msedge › tests/pageEditor/addStarterBrick.spec.ts › Add starter brick to mod

Skipped tests

chrome › tests/regressions/doNotCloseSidebarOnPageEditorSave.spec.ts › #8104: Do not automatically close the sidebar when saving in the Page Editor
msedge › tests/regressions/doNotCloseSidebarOnPageEditorSave.spec.ts › #8104: Do not automatically close the sidebar when saving in the Page Editor
chrome › tests/runtime/googleSheetsIntegration.spec.ts › can activate a google spreadsheet mod with config options
msedge › tests/runtime/googleSheetsIntegration.spec.ts › can activate a google spreadsheet mod with config options

twschiller added 16 commits September 11, 2024 17:25

[WIP] type and migration changes

d271de3

Fix migrations

7f04fea

Fix comment

023a27d

Merge remote-tracking branch 'origin/main' into feature/9137-variable…

18fe24f

…-migration

Add migration test

b97e0b3

Strengthen return type

ee5cfb3

[WIP] Page Editor save logic

b900a33

#9137: fix broken tests

8ec0434

Merge remote-tracking branch 'origin/main' into feature/9137-variable…

465e19a

…-migration

#9137: fix playwright test

7c61e42

Try fixing snapshot

984cbec

Rename variablesDefinition

de5c305

Merge remote-tracking branch 'origin/main' into feature/9137-variable…

cf862d0

…-migration

Try fixing yaml

f427c75

Remove trailing newline

5bed175

Update snapshots

73568c6

twschiller added the do not merge Merging of this PR is blocked by another one label Sep 20, 2024

twschiller self-assigned this Sep 20, 2024

twschiller commented Sep 20, 2024

View reviewed changes

src/platform/state/sessionStateHelpers.ts Outdated Show resolved Hide resolved

twschiller added the enhancement New feature or request label Sep 20, 2024

twschiller added this to the 2.1.3 milestone Sep 20, 2024

twschiller changed the title ~~#9156: tab synchronized mod variable storage~~ #9156: [WIP] tab synchronized mod variable storage Sep 20, 2024

vercel bot deployed to Preview September 21, 2024 12:15 View deployment

vercel bot deployed to Preview September 21, 2024 12:32 View deployment

twschiller changed the title ~~#9156: [WIP] tab synchronized mod variable storage~~ #9156: [WIP] tab synchronized mod variable storage (3/3) Sep 21, 2024

Merge remote-tracking branch 'origin/main' into feature/9137-variable…

39b6ae7

…-migration

vercel bot deployed to Preview September 21, 2024 13:57 View deployment

twschiller force-pushed the feature/9154-session-sync branch from 6fd6e3e to 006640c Compare September 21, 2024 14:02

twschiller force-pushed the feature/9156-tab-sync-policy branch from 5a3f887 to ef073a8 Compare September 21, 2024 14:43

vercel bot deployed to Preview September 21, 2024 14:44 View deployment

vercel bot deployed to Preview September 21, 2024 15:36 View deployment

twschiller requested a review from grahamlangford September 21, 2024 15:37

twschiller changed the title ~~#9156: [WIP] tab synchronized mod variable storage (3/3)~~ #9156: tab synchronized mod variable storage (3/3) Sep 21, 2024

twschiller marked this pull request as ready for review September 21, 2024 15:37

twschiller requested a review from fungairino September 21, 2024 15:44

vercel bot deployed to Preview September 21, 2024 15:48 View deployment

grahamlangford reviewed Sep 23, 2024

View reviewed changes

src/contentScript/stateController/stateController.test.ts Outdated Show resolved Hide resolved

grahamlangford approved these changes Sep 23, 2024

View reviewed changes

twschiller force-pushed the feature/9154-session-sync branch from 117ef4b to b25b38d Compare September 23, 2024 18:52

Base automatically changed from feature/9154-session-sync to main September 23, 2024 19:51

twschiller added 4 commits September 23, 2024 17:53

Mod variable session sync

6f3de2e

Add state controller test 9154: add E2E runtime test Improve comments Refactor dipatch method Fix type errors

Fix overzealous exception

8ba3b9b

Tab variable storage

8743519

Improve method names Add jest tests Split up state controller Remove performance note

Merge with main

b925568

twschiller force-pushed the feature/9156-tab-sync-policy branch from 5ead239 to b925568 Compare September 23, 2024 22:37

vercel bot deployed to Preview September 23, 2024 22:38 View deployment

PR feedback on test improvements

86d0070

twschiller enabled auto-merge (squash) September 23, 2024 22:45

vercel bot deployed to Preview September 23, 2024 22:46 View deployment

twschiller merged commit 9d4cf78 into main Sep 23, 2024
20 checks passed

twschiller deleted the feature/9156-tab-sync-policy branch September 23, 2024 23:15

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

#9156: tab synchronized mod variable storage (3/3) #9181

#9156: tab synchronized mod variable storage (3/3) #9181

twschiller commented Sep 20, 2024 •

edited

Loading

grahamlangford Sep 23, 2024

twschiller Sep 23, 2024

github-actions bot commented Sep 23, 2024

codecov bot commented Sep 23, 2024 •

edited

Loading

github-actions bot commented Sep 23, 2024 •

edited

Loading

#9156: tab synchronized mod variable storage (3/3) #9181

#9156: tab synchronized mod variable storage (3/3) #9181

Conversation

twschiller commented Sep 20, 2024 • edited Loading

What does this PR do?

Remaining Work

Demo

Future Work

grahamlangford Sep 23, 2024

Choose a reason for hiding this comment

twschiller Sep 23, 2024

Choose a reason for hiding this comment

github-actions bot commented Sep 23, 2024

codecov bot commented Sep 23, 2024 • edited Loading

Codecov Report

github-actions bot commented Sep 23, 2024 • edited Loading

Playwright test results

Details

twschiller commented Sep 20, 2024 •

edited

Loading

codecov bot commented Sep 23, 2024 •

edited

Loading

github-actions bot commented Sep 23, 2024 •

edited

Loading