Skip to content

fixes several sonar code-smell for no assertion in test.#3379

Merged
zhumin8 merged 6 commits intomasterfrom
asserts2
Aug 4, 2021
Merged

fixes several sonar code-smell for no assertion in test.#3379
zhumin8 merged 6 commits intomasterfrom
asserts2

Conversation

@zhumin8
Copy link
Copy Markdown
Contributor

@zhumin8 zhumin8 commented Aug 3, 2021

@google-cla google-cla bot added the cla: yes label Aug 3, 2021
@zhumin8 zhumin8 requested review from chanseokoh and suztomo August 3, 2021 22:23
@zhumin8 zhumin8 marked this pull request as ready for review August 3, 2021 22:24
@chanseokoh chanseokoh requested a review from mpeddada1 August 3, 2021 22:35
@zhumin8
Copy link
Copy Markdown
Contributor Author

zhumin8 commented Aug 3, 2021

Adding some context: I only found about this attribute in this stack-overflow answer.
Briefly read here and junit javadoc describes Test.None as default empty exception.
Let me know what do you think about this approach?


mapper.readValue(data, FilePropertiesSpec.class);
// pass
FilePropertiesSpec parsed = mapper.readValue(data, FilePropertiesSpec.class);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I’m more familiar with these types of questions on values rather than the “Test.None.class” approach.

}

@Test
@Test(expected = Test.None.class /* no exception expected */)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This is an interesting discovery and approach! But maybe let's stick with directly calling assert statements to follow the guidelines on having dedicated arrange, act and assert blocks in tests?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

make sense. Let me do the changes and stick to assert statements for these.

SkaffoldSyncMapTemplate templateNoGenerated =
SkaffoldSyncMapTemplate.from(TEST_JSON_NO_GENERATED);
Assert.assertTrue(templateNoGenerated.getGenerated().isEmpty());
// pass if no exceptions
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

We can remove this comment now

@mpeddada1
Copy link
Copy Markdown
Contributor

LGTM, just a few minor comments.

Co-authored-by: Mridula <66699525+mpeddada1@users.noreply.github.com>
@sonarqubecloud
Copy link
Copy Markdown

sonarqubecloud bot commented Aug 4, 2021

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@zhumin8 zhumin8 merged commit 79f3af7 into master Aug 4, 2021
@zhumin8 zhumin8 deleted the asserts2 branch August 4, 2021 18:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants