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

[SYNTH-16456] Make server result optional in Result #1483

Merged

Conversation

Drarig29
Copy link
Contributor

What and why?

The source of truth for a result is the batch and not the server result (which we poll with the /poll_results endpoint).

The server result corresponding to a batch result may happen to never be available, and in that case datadog-ci should fall back to the source of truth (the batch) instead of failing.

When such thing happens for a result, a warning log is printed (The information for result {resultId} of test {publicId} was incomplete at the end of the batch.) and the Result.result property will be undefined.

How?

  • Make Result.result optional
  • Update fixtures and tests

Review checklist

  • Feature or bugfix MUST have appropriate tests (unit, integration)

@Drarig29 Drarig29 added the synthetics Related to [synthetics] label Oct 28, 2024
@Drarig29 Drarig29 marked this pull request as ready for review October 28, 2024 17:24
@Drarig29 Drarig29 requested review from a team as code owners October 28, 2024 17:24
@datadog-datadog-prod-us1
Copy link

datadog-datadog-prod-us1 bot commented Oct 29, 2024

Datadog Report

Branch report: corentin.girard/SYNTH-16456/make-server-result-optional
Commit report: 3a2b2d8
Test service: datadog-ci-tests

✅ 0 Failed, 1636 Passed, 0 Skipped, 1m 54.37s Total duration (1m 35.44s time saved)

Copy link
Contributor

@etnbrd etnbrd left a comment

Choose a reason for hiding this comment

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

Thanks for adding the comments! 🙇
Added some precision on the comment for hasDefinedResult about missing result.
Otherwise, LGTM!

src/commands/synthetics/utils/internal.ts Show resolved Hide resolved
@Drarig29 Drarig29 merged commit ca6664b into master Oct 29, 2024
18 checks passed
@Drarig29 Drarig29 deleted the corentin.girard/SYNTH-16456/make-server-result-optional branch October 29, 2024 13:48
@Drarig29 Drarig29 mentioned this pull request Oct 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
synthetics Related to [synthetics]
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants