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

[datadog_synthetics_test] update the step params when reordering steps #2741

Merged

Conversation

etnbrd
Copy link
Contributor

@etnbrd etnbrd commented Dec 24, 2024

When conciliating the config and the state, the provider is not updating the multiLocator (ML) in the state as a side effect of the diffSuppressFunc, but it nonetheless updates the other fields. So after reordering the steps in the config, the state contains steps with mixed up MLs. This propagates to the crafted request to update the test on the backend, and eventually mess up the remote test.

To fix this issue, this PR allows the user to provide a local key for each step to track steps when reordering.
The provider can use this local key to reconcile the right ML into the right step.

@etnbrd etnbrd force-pushed the etnbrd/SYNTH-14958/allow-browser-steps-reordering branch 9 times, most recently from e2c16ed to ab77427 Compare December 27, 2024 17:26
@etnbrd etnbrd marked this pull request as ready for review December 27, 2024 17:33
@etnbrd etnbrd requested review from a team as code owners December 27, 2024 17:33
@etnbrd etnbrd changed the title update the ML in the steps when reordering [synthetic_test] update the step params when reordering steps Dec 27, 2024
@etnbrd etnbrd force-pushed the etnbrd/SYNTH-14958/allow-browser-steps-reordering branch from ab77427 to f0099c6 Compare December 27, 2024 17:40
estherk15
estherk15 previously approved these changes Dec 27, 2024
docs/resources/synthetics_test.md Outdated Show resolved Hide resolved
@nkzou nkzou changed the title [synthetic_test] update the step params when reordering steps [datadog_synthetics_test] update the step params when reordering steps Dec 27, 2024
Copy link
Contributor

@teodor2312 teodor2312 left a comment

Choose a reason for hiding this comment

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

Some question but otherwise LGTM 👍

datadog/resource_datadog_synthetics_test_.go Show resolved Hide resolved
datadog/resource_datadog_synthetics_test_.go Show resolved Hide resolved
datadog/resource_datadog_synthetics_test_.go Show resolved Hide resolved
@etnbrd
Copy link
Contributor Author

etnbrd commented Jan 6, 2025

/merge

@dd-devflow
Copy link

dd-devflow bot commented Jan 6, 2025

Devflow running: /merge

View all feedbacks in Devflow UI.


2025-01-06 17:41:04 UTC ℹ️ MergeQueue: pull request added to the queue

The median merge time in master is 1m.


2025-01-06 17:42:31 UTC ℹ️ MergeQueue: This merge request was merged

@dd-mergequeue dd-mergequeue bot merged commit f421aba into master Jan 6, 2025
17 checks passed
@dd-mergequeue dd-mergequeue bot deleted the etnbrd/SYNTH-14958/allow-browser-steps-reordering branch January 6, 2025 17:42
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.

5 participants