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

Explicitly test for empty distribution test changelist #2869

Merged
merged 3 commits into from
Jan 31, 2023

Conversation

WardBrian
Copy link
Member

Follow on to #2864 after test failures in #2846. It seems that the list of changes being empty still tried to run some tests (I assumed the for loop would just never execute) so this is making that conditional explicit.

syclik
syclik previously approved these changes Jan 30, 2023
Copy link
Member

@syclik syclik left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@serban-nicusor-toptal serban-nicusor-toptal left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks

@WardBrian
Copy link
Member Author

@serban-nicusor-toptal this still didn't work - is there a good way to print a Groovy variable for debugging? For some reason I guess the list isn't empty but also doesn't contain a test, so I'm worried the value is like [""]

@WardBrian
Copy link
Member Author

Okay, I think I figured it out. Using .readLines() rather than .split('\n').toList() seems to do what we want when the string is only whitespace

@WardBrian WardBrian merged commit 9149f05 into develop Jan 31, 2023
@WardBrian WardBrian deleted the tests/empty-distribution-changeset branch January 31, 2023 13:15
@serban-nicusor-toptal
Copy link
Contributor

println inside a script block, I saw that you already figured it out. Sorry, but I saw the message now.

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.

None yet

3 participants