-
Notifications
You must be signed in to change notification settings - Fork 21
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
messaging: page-world bridge #1213
Conversation
Temporary Branch UpdateThe temporary branch has been updated with the latest changes. Below are the details:
Please use the above install command to update to the latest version. |
[Beta] Generated file diffTime updated: Fri, 15 Nov 2024 18:47:54 GMT Android
File has changed Chrome
File has changed Chrome-mv3
File has changed Firefox
File has changed Integration
File has changed Windows
File has changed Apple
File has changed |
✅ Deploy Preview for content-scope-scripts ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
injected/src/features/message-bridge/create-page-world-bridge.js
Outdated
Show resolved
Hide resolved
injected/src/features/message-bridge/create-page-world-bridge.js
Outdated
Show resolved
Hide resolved
injected/src/features/message-bridge/create-page-world-bridge.js
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The code looks good! I just want to make sure that we give the config structure enough time.
injected/src/features/message-bridge/create-page-world-bridge.js
Outdated
Show resolved
Hide resolved
injected/src/features/message-bridge/create-page-world-bridge.js
Outdated
Show resolved
Hide resolved
injected/src/features/message-bridge/create-page-world-bridge.js
Outdated
Show resolved
Hide resolved
injected/src/features/message-bridge/create-page-world-bridge.js
Outdated
Show resolved
Hide resolved
injected/src/features/message-bridge/create-page-world-bridge.js
Outdated
Show resolved
Hide resolved
I commented on it, maybe it got lost when I pushed more code?
Basically this is the version that will allow all the others to be cleaned
up, so it’s a first step in that direction, I’ll fast follow with
removing all the rest.
On November 15, 2024, GitHub notifications ***@***.***> wrote:
@jonathanKingston commented on this pull request.
In injected/integration-test/page-objects/results-collector.js
<https://github.com/duckduckgo/content-scope-
scripts/pull/1213#discussion_r1844482796>:
> @@ -0,0 +1,200 @@ +import { readFileSync } from 'fs'; +import { +
mockAndroidMessaging, + mockWebkitMessaging, + mockWindowsMessaging, +
simulateSubscriptionMessage, + wrapWebkitScripts, +
wrapWindowsScripts, +} from ***@***.***/messaging/lib/test-
utils.mjs'; +import { perPlatform } from '../type-helpers.mjs'; + +/**
+ * This is designed to allow you to execute Playwright tests using
the various
Why do we keep copying this boilerplate over and over? I get that
there are subtle differences but overall this is a lot of code for how
small the actual test is.
Can we file an issue to clean this up at the minimum?
—
Reply to this email directly, view it on GitHub
<https://github.com/duckduckgo/content-scope-
scripts/pull/1213#pullrequestreview-2439646350>, or unsubscribe
<https://github.com/notifications/unsubscribe-
auth/AAMRIAR2FQBYQJ3ICEVFQ6T2AZOE3AVCNFSM6AAAAABRSIOBKKVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDIMZZGY2DMMZVGA>.
You are receiving this because you were mentioned.Message ID:
<duckduckgo/content-scope-
***@***.***>
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's a few lines we can remove that are commented out.
However we really need to get on top of the boilerplate we're amassing in the tests; can you file a follow up please?
Cool yeah do we need to rethink them all being mapped into that pages test? Happy to cover in a follow up anyway :) |
Yeah, we should just have 2 boiler plates. Extension setup + all the rest. I actually have a branch with this all sorted so I will bring it back to life now that this is getting worse. |
I'll merge this, and follow immediately with boilerplate removal where possible. |
Asana Task/Github Issue: https://app.asana.com/0/1201141132935289/1208740665800207/f
Description
messageBridge
(apple-isolated only for now)createMessageBridge
onnavigatorInterface
- to return the MessagingInterfaceTesting Steps
Checklist
Please tick all that apply: