Mobile/amp tests: Potential fixes for 4 code quality findings - #359
Mobile/amp tests: Potential fixes for 4 code quality findings#359patmmccann wants to merge 4 commits into
Conversation
…lot Autofix Co-authored-by: Copilot Autofix powered by AI <223894421+github-code-quality[bot]@users.noreply.github.com>
…lot Autofix Co-authored-by: Copilot Autofix powered by AI <223894421+github-code-quality[bot]@users.noreply.github.com>
…lot Autofix Co-authored-by: Copilot Autofix powered by AI <223894421+github-code-quality[bot]@users.noreply.github.com>
…lot Autofix Co-authored-by: Copilot Autofix powered by AI <223894421+github-code-quality[bot]@users.noreply.github.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0d1e4934fc
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const markup = '<script>window.testScriptExecuted=true;</script>' | ||
| writeAdHtml(markup); | ||
| const ps = sinon.stub(); | ||
| writeAdHtml(markup, ps); |
There was a problem hiding this comment.
Use real postscribe when asserting script execution
Passing a sinon.stub() here means writeAdHtml only calls the fake postscribe function; it no longer appends the markup or executes the inline script. In the Karma browser test, window.testScriptExecuted therefore remains unset and the newly added assertion setup makes this test fail whenever the suite runs. Keep the real postscribe path for the execution check, or have the fake actually insert/evaluate the markup.
Useful? React with 👍 / 👎.
This PR applies 4/4 suggestions from code quality AI findings.