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

Update manage to stop on agent failure #886

Merged

Conversation

nodlesh
Copy link
Contributor

@nodlesh nodlesh commented Oct 29, 2024

When running a set of tests, and the start routine fails to communicate to one of the designated agents, the execution of the tests would continue. Of course this would cause some or all of the tests to fail. This PR changes that logic and when the test harness does not get a successful startup of an agent, will let the user know there was an issue, skip the running of the tests, and stop any running test harness services. This should end the interop results getting complete failures in the reports when it was just an agent startup issue.

Signed-off-by: Sheldon Regular <sheldon.regular@gmail.com>
Copy link
Contributor

@swcurran swcurran left a comment

Choose a reason for hiding this comment

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

Nice fix — that one has bugged me for a while, but I just accepted it :-).

@swcurran swcurran merged commit b4e90bb into openwallet-foundation:main Nov 6, 2024
2 checks passed
@nodlesh
Copy link
Contributor Author

nodlesh commented Nov 6, 2024

There is an issue with the credo agent I've been working on for a few days now. So this PR stopped the running of tests last night. However the job to push results to Allure ran and it shows empty in the report. Do you think this is ok, or should we skip the results push if tests don't run. It is an indicator that something went wrong and no tests were executed I suppose. I'm leaning to avoiding a results push if tests are skipped. Agreed?

@swcurran
Copy link
Contributor

swcurran commented Nov 6, 2024

Yes, I think that makes sense.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants