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

Fix: unwrap event & fix groups typo. #14

Merged
merged 3 commits into from
May 11, 2022

Conversation

laurentsenta
Copy link
Contributor

@laurentsenta laurentsenta commented May 2, 2022

When testing with the latest testground, the example would end in "failed" state despite a successful run. When I looked at the JSON, it looked like the payload for events are malformed and they are not registered by the testground runner.

This changes the payload from:

{"event":{"success_event":{"groups":"single"}}}

to:

{"success_event":{"group":"single"}}

@laurentsenta laurentsenta marked this pull request as draft May 2, 2022 15:21
@@ -20,9 +20,9 @@ pub enum EventType {
#[serde(rename = "message_event")]
Message { message: String },
#[serde(rename = "success_event")]
Success { groups: String },
Success { group: String },
Copy link
Member

Choose a reason for hiding this comment

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

@mxinden
Copy link
Member

mxinden commented May 2, 2022

Mind adding a changelog entry?

https://github.com/testground/sdk-rust/blob/master/CHANGELOG.md

Also is there any way to make the CI fail on this error, to prevent regressions in the future?

- name: Run testground plan (sdk-rust)
run: |
testground run single \
--plan=sdk-rust \
--testcase=example \
--builder=docker:generic \
--runner=local:docker \
--instances=1 \
--wait

@laurentsenta
Copy link
Contributor Author

laurentsenta commented May 2, 2022

(changelog updated and follow-up issue created in pl-strflt/ipdx#20)

@laurentsenta laurentsenta requested a review from mxinden May 3, 2022 06:20
@mxinden
Copy link
Member

mxinden commented May 4, 2022

Mhhh, with current master I get:

May  4 15:37:32.920913  INFO    run finished successfully       {"run_id": "c9p9qe4oi0mko9e6i0l0", "plan": "sdk-rust", "case": "example", "runner": "local:docker", "instances": 1}

with this branch I get:

May  4 15:33:49.226364  WARN    run finished in error   {"run_id": "c9p9pjkoi0mko9e6i0kg", "plan": "sdk-rust", "case": "example", "runner": "local:docker", "instances": 1, "error": "1 nodes failed"}  

Testground version:

$ testground  version  
Testground
Git commit: 6ae5c95b

Still need to debug this further.

@laurentsenta
Copy link
Contributor Author

laurentsenta commented May 10, 2022

Could you run the test a couple of times and share the full log when this branch fails? (I couldn't reproduce),

run finished successfully is expected in both cases,

That's the dashboard result we should care about:
if you look at http://localhost:8042 I suspect the test run on master will have status=❌
and the test in this branch will pass most of the time.

@laurentsenta
Copy link
Contributor Author

@mxinden fwiw running on the CI (with the other patch that captures failures correctly):

master fails consistently:
https://github.com/laurentsenta/sdk-rust/actions/runs/2301527886
(top right, there is a dropdown with all 4 failed runs)

this branch passes:
https://github.com/laurentsenta/sdk-rust/actions/runs/2301540056
(same dropdown top right, "most" runs pass)

Copy link
Member

@mxinden mxinden left a comment

Choose a reason for hiding this comment

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

Not able to reproduce the failures I have been seeing. Sorry for the noise.

Tested once again locally. Both log output and dashboard report run with this branch as success.

Will merge here. Thanks @laurentsenta!

@mxinden mxinden merged commit 054d83e into testground:master May 11, 2022
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.

None yet

2 participants