Skip to content
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

[Lighthouse Flake]: No node found for selector: mat-option[ng-reflect-value="ADMIN"] #17279

Open
U8NWXD opened this issue Feb 8, 2023 · 14 comments
Labels
bug Label to indicate an issue is a regression CI breakage This bug breaks CI workflows. Impact: Medium Will improve quality-of-life for at least 30% of users. Work: Medium The means to find the solution is clear, but it isn't at good-first-issue level yet.

Comments

@U8NWXD
Copy link
Member

U8NWXD commented Feb 8, 2023

Log:

Running Lighthouse 3 time(s) on http://127.0.0.1:8181/donate
Run #1...done.
Run #2...done.
Run #3...done.
Error: No node found for selector: mat-option[ng-reflect-value="ADMIN"]
    at assert (/home/runner/work/oppia/oppia/node_modules/puppeteer/lib/cjs/puppeteer/common/assert.js:26:15)
    at DOMWorld.click (/home/runner/work/oppia/oppia/node_modules/puppeteer/lib/cjs/puppeteer/common/DOMWorld.js:287:32)
    at runMicrotasks (<anonymous>)
    at processTicksAndRejections (node:internal/process/task_queues:96:5)
    at async setRole (/home/runner/work/oppia/oppia/puppeteer-login-script.js:103:5)
    at async module.exports (/home/runner/work/oppia/oppia/puppeteer-login-script.js:47:5)
    at async PuppeteerManager.invokePuppeteerScriptForUrl (/home/runner/work/oppia/oppia/node_modules/@lhci/cli/src/collect/puppeteer-manager.js:112:5)
    at async Object.runCommand (/home/runner/work/oppia/oppia/node_modules/@lhci/cli/src/collect/collect.js:244:7)
    at async run (/home/runner/work/oppia/oppia/node_modules/@lhci/cli/src/cli.js:103:7)
Running Lighthouse 3 time(s) on http://127.0.0.1:8181/emaildashboard

Examples:

This flake hasn't been seen before, so it was probably introduced in the last few days. Here are the PRs merged over the last 5 days:

Despite the error in the output being No node found for selector: mat-option[ng-reflect-value="ADMIN"], this appears to be related to donorbox. The Lighthouse reports say:

  • 2023-02-28:
    Screenshot 2023-03-12 at 13 25 53

  • 2023-03-11:
    Screenshot 2023-03-12 at 13 26 05

This looks to be an issue with donorbox rate limiting us

@U8NWXD U8NWXD added bug Label to indicate an issue is a regression Impact: High Blocks or significantly slows down a core workflow. Work: Medium The means to find the solution is clear, but it isn't at good-first-issue level yet. labels Feb 25, 2023
@U8NWXD
Copy link
Member Author

U8NWXD commented Mar 12, 2023

@kevintab95 it looks like this flake might be related to the issue you hotfixed with #17661. The hotfix didn't fix it, but now more pages are failing due to a 403 error from donorbox

@kevintab95
Copy link
Member

Hi @U8NWXD, just to clarify, I think we'll need to have a "dev mode" flag for donorbox and not fetch from their domain when running e2e tests. The hotfix wasn't meant to fix a flake, the donation flow wasn't working as expected so we tried to fix that.

@U8NWXD
Copy link
Member Author

U8NWXD commented Mar 12, 2023

Thanks! I understood that your hotfix wasn't about the flake--I just wanted to loop you in since it looked related.

Any reason you prefer skipping donorbox during the tests rather than ignoring this particular error? I'm worried that if we cut donorbox out of the tests entirely, we'll miss bugs with it.

Also /cc @seanlip since we're considering reducing test coverage

@seanlip
Copy link
Member

seanlip commented Mar 12, 2023

I approve skipping donorbox during the tests -- I think there's a bit of an issue with exercising an external prod dependency multiple times a day due to our tests and on balance we probably shouldn't do it.

(We've actually already missed bugs even with the tests anyway.)

Will probably need to earmark this flow for manual testing on each release -- I will have a chat with @kevintab95 about that.

@U8NWXD
Copy link
Member Author

U8NWXD commented Mar 12, 2023

OK, sounds good. Any suggestions on how to skip donorbox during the tests? Note that many tests run in prod mode, so that flag won't be enough

@U8NWXD
Copy link
Member Author

U8NWXD commented Mar 12, 2023

I also agree with manual testing on each release. The tests have missed donorbox bugs, but if we don't test it at all, we should count on missing even more

@seanlip
Copy link
Member

seanlip commented Mar 13, 2023

Any suggestions on how to skip donorbox during the tests? Note that many tests run in prod mode, so that flag won't be enough

Hm, I'm not familiar enough with this part -- @kevintab95 might need to comment here.

@kevintab95
Copy link
Member

kevintab95 commented Mar 16, 2023

I think there are a couple of things to do here to fully solve this:

  1. Fix [TODO]: Fetch widget.js from donorbox only in the donate page #17664 so that we limit fetching widget.js to the donate page only.
  2. Create a new flag similar to CAN_SEND_ANALYTICS_EVENTS (e.g. DONORBOX_IS_SHOWN). Should be OK to repurpose analytics-constants.json for this if necessary. Use this flag to show / hide the script tag in the HTML.
  3. Update the release scripts such that the constant is updated correctly depending on the environment.

@gp201
Copy link
Member

gp201 commented Apr 22, 2023

Dropping this flake to priority level since we haven't seen this flake since the beginning of April.

@gp201 gp201 added Impact: Medium Will improve quality-of-life for at least 30% of users. and removed Impact: High Blocks or significantly slows down a core workflow. labels May 23, 2023
@462702985
Copy link
Contributor

462702985 commented Jun 15, 2023

@gp201 gp201 added Impact: High Blocks or significantly slows down a core workflow. and removed Impact: Medium Will improve quality-of-life for at least 30% of users. labels Jun 17, 2023
@gp201
Copy link
Member

gp201 commented Jun 17, 2023

This issue has been moved back to high priority since @462702985 has identified that the flake has been occurring at a high rate. Thanks for catching this @462702985

image

@seanlip
Copy link
Member

seanlip commented Jun 18, 2023

Hi -- just a note. This looks like it is due to console errors in the /donate page which are caused by an error in a third-party component (stripe). See https://storage.googleapis.com/lighthouse-infrastructure.appspot.com/reports/1687048851060-5704.report.html and look at the accordion under "Browser errors were logged to the console".
Screenshot from 2023-06-18 18-58-11

We should update our lighthouse config to try and exclude those errors. I saw GoogleChrome/lighthouse#9480 but I can't quite figure out how to modify .lighthouserc-1.js or .lighthouserc-base.js to include the ignoredPatterns option (my first attempt didn't work). I suggest trying to do this or just reducing the threshold for now so that PRs can go in.

@462702985
Copy link
Contributor

@seanlip I didn't find where we can add ignoredPatterns too. And I tried to add Content Security Policy but I cannot find http header of stripe.com.

@seanlip seanlip added Impact: Medium Will improve quality-of-life for at least 30% of users. and removed Impact: High Blocks or significantly slows down a core workflow. labels Dec 1, 2023
@seanlip seanlip added the CI breakage This bug breaks CI workflows. label Mar 13, 2024
@seanlip
Copy link
Member

seanlip commented Mar 13, 2024

Note, this is causing consistent failures on Docker -- it needs to be investigated.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Label to indicate an issue is a regression CI breakage This bug breaks CI workflows. Impact: Medium Will improve quality-of-life for at least 30% of users. Work: Medium The means to find the solution is clear, but it isn't at good-first-issue level yet.
Projects
Status: Todo
Status: Todo
Development

No branches or pull requests

6 participants