feat: allow times to be <= 0 to simulate endless trigger

[youjie23]

Since some existing test cases rely on the behavior described in #132, we need to maintain compatibility with them to avoid extensive modifications to the e2e tests. Correspondingly, we will only modify the necessary test cases by explicitly setting the stop-on-reach-times configuration to true

[wu-sheng]

I think we need to update docs about this config?

[youjie23]

I think we need to update docs about this config?

I've updated the corresponding documentation. Could you please take another look when you have a moment to see if it meets the requirements

[Copilot]

Pull Request Overview

This PR adds support for a stop-on-reach-times configuration option in the trigger phase to control whether the trigger should stop when the retry count is reached. The default value is false to maintain backward compatibility with existing test cases that rely on the previous behavior (as described in #132).

Key changes:

• Added stop-on-reach-times configuration field with a default value of false
• Modified the HTTP trigger action to respect this configuration when deciding whether to stop execution
• Updated documentation to reflect the new configuration option

Reviewed Changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.

File	Description
internal/config/e2eConfig.go	Added StopOnReachTimes boolean field to the Trigger struct
internal/components/trigger/http.go	Added stopOnReachTimes field and logic to conditionally stop execution when times is reached
docs/en/setup/Configuration-File.md	Documented the new stop-on-reach-times configuration option with default value
commands/trigger/trigger.go	Passed the StopOnReachTimes value to the HTTP action constructor

---

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

[kezhenxu94]

That looks weird to me. Do we need to fix many test cases? Can we fix them with some script?

[youjie23]

That looks weird to me. Do we need to fix many test cases? Can we fix them with some script?

After the GHA run, I've identified approximately 10 test cases that require modification and I suspect there might be more to it.

On one hand, we could simply set the times value in all trigger configurations to a very high number. This would make the triggers run continuously throughout the entire end-to-end test verification period, behave as before. However, this adjustment seems a bit odd, doesn't it??

On the other hand, having the trigger run continuously is acceptable and works fine in other scenarios. The specific issue here is with the ​​alert recovery detection scenario​​(#13539). This scenario requires a defined recovery time window, meaning the trigger needs to stop after a certain number of runs to accurately assess recovery.

Therefore, my proposal is to ​​add a new configuration option​​ to explicitly control whether the trigger stops after reaching the specified times value. This provides the necessary flexibility for the alert recovery use case without affecting other scenarios.

Looking forward to your suggestions on this approach.

[kezhenxu94]

The times is expected to control how many times the call should be triggered, the test case you mentioned worked before because of this bug, and since we fixed this bug we should adjust the test case instead of adding a new opposite config to make it work again.

On the other hand, having the trigger run continuously is acceptable and works fine in other scenarios. The specific issue here is with the ​​alert recovery detection scenario​​(#13539). This scenario requires a defined recovery time window, meaning the trigger needs to stop after a certain number of runs to accurately assess recovery.

I think we can set the times to a non-positive value like (-1 or 0) to indicate never stop (if this doesn't already exist). Instead of adding a config stop-on-reach-times that had been expected to work with the times field.

[youjie23]

The times is expected to control how many times the call should be triggered, the test case you mentioned worked before because of this bug, and since we fixed this bug we should adjust the test case instead of adding a new opposite config to make it work again.

On the other hand, having the trigger run continuously is acceptable and works fine in other scenarios. The specific issue here is with the ​​alert recovery detection scenario​​(#13539). This scenario requires a defined recovery time window, meaning the trigger needs to stop after a certain number of runs to accurately assess recovery.

I think we can set the times to a non-positive value like (-1 or 0) to indicate never stop (if this doesn't already exist). Instead of adding a config stop-on-reach-times that had been expected to work with the times field.

Thanks for the feedback! I'll work on a revision and submit it later.

[youjie23]

The times is expected to control how many times the call should be triggered, the test case you mentioned worked before because of this bug, and since we fixed this bug we should adjust the test case instead of adding a new opposite config to make it work again.

On the other hand, having the trigger run continuously is acceptable and works fine in other scenarios. The specific issue here is with the ​​alert recovery detection scenario​​(#13539). This scenario requires a defined recovery time window, meaning the trigger needs to stop after a certain number of runs to accurately assess recovery.

I think we can set the times to a non-positive value like (-1 or 0) to indicate never stop (if this doesn't already exist). Instead of adding a config stop-on-reach-times that had been expected to work with the times field.

I've pushed new commits to address the points you raised. Please take another look when you have a moment, and let me know if anything else needs adjustment. @kezhenxu94

[kezhenxu94]

Thanks!