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

test(scorecard): adjust scorecard report generation expectation #985

Merged
merged 2 commits into from
Dec 17, 2024

Conversation

andrewazores
Copy link
Member

@andrewazores andrewazores commented Dec 17, 2024

Welcome to Cryostat! 👋

Before contributing, make sure you have:

  • Read the contributing guidelines
  • Linked a relevant issue which this PR resolves
  • Linked any other relevant issues, PR's, or documentation, if any
  • Resolved all conflicts, if any
  • Rebased your branch PR on top of the latest upstream main branch
  • Attached at least one of the following labels to the PR: [chore, ci, docs, feat, fix, test]
  • Signed all commits: git commit -S -m "YOUR_COMMIT_MESSAGE"

See cryostatio/cryostat#698
See #974

Description of the change:

Adjusts the test to not expect a full report JSON response body, on request, but only a job ID string response.

Since cryostatio/cryostat#698 the response is expected to be an async job ID on first request, then later the server will emit a websocket notification containing the same ID to notify clients that the request has been completed. Then when the client makes an identical request again, the report is ready and cached, so the server responds with it this second time around.

In the old system the server would always respond with the report response, even though this could sometimes take considerable time to complete. Because of this long delay, with the server writing no response bytes at all in the interim, clients (especially web browsers) could time out waiting while the server was still processing.

Ideally this scorecard test should be adjusted to account for the entire async job API cycle, but this would mean establishing a websocket connection to the server to listen for the async job ID. I think this is worth looking into but I will do it separately, and just get this minimal fix in ASAP to get CI passing again.

Motivation for the change:

Get scorecards passing again.

How to manually test:

  1. Insert steps here...
  2. ...

@andrewazores andrewazores changed the title fix(scorecard): adjust scorecard report generation expectation test(scorecard): adjust scorecard report generation expectation Dec 17, 2024
@andrewazores
Copy link
Member Author

/build_test

Copy link

/build_test completed successfully ✅.
View Actions Run.

@andrewazores andrewazores marked this pull request as ready for review December 17, 2024 20:37
@andrewazores andrewazores requested a review from a team December 17, 2024 20:37
Copy link
Member

@ebaron ebaron left a comment

Choose a reason for hiding this comment

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

Looks good, would you mind filing an issue to test the full async report generation?

@andrewazores
Copy link
Member Author

#986

@andrewazores andrewazores merged commit a543918 into cryostatio:main Dec 17, 2024
11 checks passed
@andrewazores andrewazores deleted the scorecard-long-running-api branch December 17, 2024 21:07
andrewazores added a commit to andrewazores/cryostat-operator that referenced this pull request Dec 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants