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

Fix flaky tests when legacyplatform analyzer is invoked #179

Merged
merged 1 commit into from
Apr 2, 2024

Conversation

xnyo
Copy link
Member

@xnyo xnyo commented Apr 2, 2024

Fixes flaky tests involving the legacyplatform analyzer (angular) when https://grafana.com/api/plugins/angular_patterns is called too quickly:

--- FAIL: TestRunner (0.39s)
    --- FAIL: TestRunner/AllFilesPresentButEmpty (0.23s)
        runner_test.go:72: legacyplatform: json: cannot unmarshal object into Go value of type []legacyplatform.gcomPattern
FAIL
FAIL    github.com/grafana/plugin-validator/pkg/runner  0.488s

This is happening because calling the API too fast will trigger the rate limit in GCOM:

{
  "code": "TooManyRequests",
  "message": "Rate limit exceeded.",
  "requestId": "d81fb3f1-ed30-4e66-8da6-4dc7fb7a2d38"
}

With this PR, we cache the result and call the GCOM API only once

@xnyo xnyo added the bug Something isn't working label Apr 2, 2024
@xnyo xnyo self-assigned this Apr 2, 2024
@xnyo xnyo requested review from a team as code owners April 2, 2024 09:03
@xnyo xnyo requested review from wbrowne and andresmgot and removed request for a team April 2, 2024 09:03
Copy link
Contributor

@andresmgot andresmgot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need to expire the cache?

@xnyo
Copy link
Member Author

xnyo commented Apr 2, 2024

Do we need to expire the cache?

No, those hardly ever change and it this only lives in memory anyways, so re-running the plugin-validator will expire the cache.

We only need it so we don't hit GCOM API too quickly and get back a 429, mostly when running tests

Copy link
Contributor

@andresmgot andresmgot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@xnyo xnyo merged commit 2729ec9 into main Apr 2, 2024
2 checks passed
@xnyo xnyo deleted the giuseppe/fix-flaky-test branch April 2, 2024 12:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

None yet

2 participants