Skip to content

Conversation

@mivanov1988
Copy link
Contributor

This change aims to fix the NPE when null is passed to EmailNotification.concatAddresses().

Testing done: unit tests

Signed-off-by: Miroslav Ivanov [email protected]

@antoniivanov
Copy link
Contributor

This is crying for unit tests, I think :)

And I find it a good practice to have a test for each bug we fix. Bug tend to re-surface (unless we check for them). I'd usually even write the test before I make the fix (thus helping me write the fix faster and making sure I have a good test).

This change aims to fix the NPE when null is passed to EmailNotification.concatAddresses().

Testing done: unit tests

Signed-off-by: Miroslav Ivanov [email protected]
@mivanov1988 mivanov1988 force-pushed the topic/miroslavi/fix-concat-addresses branch from 25e9fef to 65787ae Compare April 5, 2022 07:13
@mivanov1988
Copy link
Contributor Author

This is crying for unit tests, I think :)

And I find it a good practice to have a test for each bug we fix. Bug tend to re-surface (unless we check for them). I'd usually even write the test before I make the fix (thus helping me write the fix faster and making sure I have a good test).

The whole class is crying for unit tests, but such have not been implemented :)

@mivanov1988
Copy link
Contributor Author

This is crying for unit tests, I think :)

And I find it a good practice to have a test for each bug we fix. Bug tend to re-surface (unless we check for them). I'd usually even write the test before I make the fix (thus helping me write the fix faster and making sure I have a good test).

Added tests.

@mivanov1988 mivanov1988 enabled auto-merge (squash) April 5, 2022 07:19
@mivanov1988 mivanov1988 merged commit 623b010 into main Apr 5, 2022
@mivanov1988 mivanov1988 deleted the topic/miroslavi/fix-concat-addresses branch April 5, 2022 07:29
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.

4 participants