Skip to content

Conversation

@timyates
Copy link
Contributor

@timyates timyates commented Jun 3, 2024

Whilst getting sonar working I spotted that we had warnings about comparing a primitive to null in the tests

The lines looked like

assertNotNull(response.getContentLength());

Which will always pass.

Also saw some places where we had missed the assertion

response.equals(opportunitiesResponse);

Which I replaced with

assertEquals(Set.of(opportunitiesResponse), response.body());

Whilst getting sonar working I spotted that we had warnings about comparing a primitive to null in the tests

The lines looked like

```
assertNotNull(response.getContentLength());
```

Which will always pass.

Also saw some places where we had missed the assertion

```
response.equals(opportunitiesResponse);
```

Which I replaced with

```
assertEquals(Set.of(opportunitiesResponse), response.body());
```
@timyates timyates requested review from mjperry91 and mkimberlin June 3, 2024 14:54
@timyates timyates self-assigned this Jun 3, 2024
Comment on lines -188 to +174
assertNotNull(response.getContentLength());
response.equals(opportunitiesResponse);
assertTrue(response.getContentLength() > 0, "response.getContentLength() > 0");
assertEquals(Set.of(opportunitiesResponse), response.body());
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is the thing that I changed here (there are 6 instances of the null check)

The rest is formatting... I wish git diff understood formatting a bit better... it never feels big enough to warrant a separate PR, but when put in here it makes it really hard to see what's going on 😿

@mkimberlin mkimberlin merged commit 550b492 into develop Jun 4, 2024
@mkimberlin mkimberlin deleted the bugfix-stop-testing-primitives-against-null branch June 4, 2024 21:02
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.

4 participants