Skip to content

Add hasNullValue and doesNotHaveNullValue to AtomicReferenceAssert#2969

Closed
0x1306e6d wants to merge 4 commits intoassertj:mainfrom
0x1306e6d:atomic-reference-has-null-value
Closed

Add hasNullValue and doesNotHaveNullValue to AtomicReferenceAssert#2969
0x1306e6d wants to merge 4 commits intoassertj:mainfrom
0x1306e6d:atomic-reference-has-null-value

Conversation

@0x1306e6d
Copy link
Copy Markdown

Check List:

Following the contributing guidelines will make it easier for us to review and accept your PR.

hasNullValue and doesNotHaveNullValue will be useful for checking whether an AtomicReference has the null value or not. These are shortcuts of hasVallue(null) and doesNotHaveValue(null) and would be useful when using package which non-null is the default since IDE may warn that hasVallue(null) receives null.

Copy link
Copy Markdown
Member

@joel-costigliola joel-costigliola left a comment

Choose a reason for hiding this comment

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

Thanks @ghkim3221, first round of code review done.

import static org.assertj.core.api.Assertions.assertThatExceptionOfType;
import static org.assertj.core.error.ShouldNotContainValue.shouldNotContainValue;

class AtomicReferenceAssert_doesNotHaveNullValue_Test {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Can you follow the testing guidelines documented in https://github.com/assertj/assertj/blob/main/CONTRIBUTING.md notably using GIVEN WHEN THEN and AssertionUtil.expectAssertionError.

this comment applies to the other test too, thanks!

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Sure!! thanks.

@joel-costigliola joel-costigliola added this to the 3.25.0 milestone Mar 2, 2023
@joel-costigliola
Copy link
Copy Markdown
Member

Integrated thanks @ghkim3221!

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