-
Notifications
You must be signed in to change notification settings - Fork 28
[in progress] scriptlets as an npm module #1757
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
base: main
Are you sure you want to change the base?
Conversation
❌ Deploy Preview for content-scope-scripts failed.
|
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: Thu, 19 Jun 2025 15:23:39 GMT Integration
File has changed Apple
File has changed |
|
||
const test = testContextForExtension(base); | ||
|
||
test.describe('Scriptlets Integration Tests', () => { |
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.
Is all this a dupe of the test-pages stuff? I see a lot of waitForTimeout too which is pretty flakey.
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.
I see it's supposed to be testing them. it seems a lot more involved than the intent here. Can you ask it to make this file more like injected/integration-test/pages.spec.js where there's no state where possible this will make most of them work in privacy-test-pages.site without having special setup code?
I also don't see it using the config in automation? Perhaps I've misread?
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.
@@ -13,13 +13,13 @@ | |||
<p>This page verifies that the remove-node-text scriptlet works properly given the <a href="../config/remove-node-text.json">config</a>.</p> | |||
|
|||
<!-- This script should have its content removed because it contains "badScriptContent" --> |
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.
Should this test just be checking for regular div text change removal? Given the abort inline script it seems a little misleading as a test?
Asana Task/Github Issue:
Description
Testing Steps
Checklist
Please tick all that apply: