Skip to content

Add unit tests for builders #543 #734

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

Merged
merged 5 commits into from
Aug 18, 2022
Merged

Conversation

onewhl
Copy link
Member

@onewhl onewhl commented Aug 17, 2022

Description

Added unit tests for SimpleNameBuilder, SimpleCommentBuilder, SimpleClusterCommentBuilder.
Unfortunately, most of the methods are private and kotlin properties not always could be mocked, so the number of tests is small.

Fixes #543

Type of Change

Please delete options that are not relevant.

  • New tests

How Has This Been Tested?

Automated Testing

Run tests in PR.

Checklist (remove irrelevant options):

This is the author self-check list

  • The change followed the style guidelines of the UTBot project
  • Self-review of the code is passed
  • The change contains enough commentaries, particularly in hard-to-understand areas
  • New documentation is provided or existed one is altered
  • No new warnings
  • New tests have been added
  • All tests pass locally with my changes

@onewhl onewhl requested a review from amandelpie August 17, 2022 10:37
Copy link
Collaborator

@amandelpie amandelpie left a comment

Choose a reason for hiding this comment

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

It's a good question, should we test private methods or not, I suggest to use reflection or change the visibility of a few methods to the package-private or internal and test private methods without mockito but after moving from Soot to JCDB

If you wish, you could play with changing visibility at this PR for 1-2 method for the SimpleNameBuilder

Please add a few tests if possible, fix the broken test and fix the issue with the mockito verision

}

@Test
fun `throws throwable if execution result is null`() {
Copy link
Collaborator

Choose a reason for hiding this comment

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

This test fails on my side with

expected: <<pre>
Test throws Throwable 
</pre>> but was: < >

Copy link
Member Author

Choose a reason for hiding this comment

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

i didn't expect these two builders have different behavior, but now we know. I fixed test

@onewhl onewhl requested a review from amandelpie August 17, 2022 14:20
@amandelpie
Copy link
Collaborator

@onewhl LGTM for me

@amandelpie amandelpie merged commit 9730501 into main Aug 18, 2022
@amandelpie amandelpie deleted the onewhl/543_unit_tests_for_builders branch August 18, 2022 08:32
denis-fokin pushed a commit that referenced this pull request Aug 18, 2022
* Configured unit tests with mockito

* Add unit tests for SimpleNameBuilderTest #543

* Add unit tests for SimpleClusterCommentBuilderTest.kt and SimpleCommentBuilderTest.kt #543

* change mockito version, fix broken test, add tests for buildDocStmts

* add tests for invokeDescription method
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

Cover summary builders with unit tests
2 participants