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

Log task start time to verify delays and modify the loop to skip intermediate state #18377

Merged
merged 1 commit into from
Nov 21, 2023

Conversation

bhavanisn
Copy link
Contributor

This add few debug information to collect in order to debug further the issue #14469 which is not reproducible locally. The issue happens due to the intermediate state set in between the testcase from Task A to Task B and exits. This commit will also let the testcase run one another time so it will skip the intermediate state.

Signed-off-by: Bhavani SN ([email protected])

@bhavanisn
Copy link
Contributor Author

@dsouzai @llxia please review

@dsouzai
Copy link
Contributor

dsouzai commented Oct 30, 2023

For posterity:

This commit will also let the testcase run one another time so it will skip the intermediate state.

The reason for doing another iteration is because if goo() returns a malformed string a second time, then it truly is a bug and will trigger the assert, since the state will remain ?. On the other hand, forcing another iteration will ensure if that if the state was still ? at the time keepOnGoing was set to false, we ensure that the state gets set to B before terminating.

In other words, even though it helps with debugging the linked issue, it is still something that the test should do in case the main cause of the issue is timing related.

Copy link
Contributor

@dsouzai dsouzai left a comment

Choose a reason for hiding this comment

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

Looks fine to me; minor change requested.

@dsouzai
Copy link
Contributor

dsouzai commented Oct 30, 2023

I'll let @llxia review and merge this since it's a testing change.

@llxia
Copy link
Contributor

llxia commented Nov 21, 2023

Copy link
Contributor

@llxia llxia left a comment

Choose a reason for hiding this comment

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

LGTM

@llxia llxia merged commit ac47064 into eclipse-openj9:master Nov 21, 2023
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.

3 participants