Skip to content

Conversation

avgprog
Copy link
Contributor

@avgprog avgprog commented Feb 18, 2021

Resolves #148. Changes done -

  • Replaced InvalidGridException with a logger warning in case a JointGridContainer only consists of a single SubGridContainer.

@avgprog avgprog requested review from ckittl and removed request for ckittl February 18, 2021 10:00
@codecov
Copy link

codecov bot commented Feb 18, 2021

Codecov Report

Merging #278 (1eb680e) into dev (bf2b182) will increase coverage by 0.04%.
The diff coverage is n/a.

Impacted file tree graph

@@             Coverage Diff              @@
##                dev     #278      +/-   ##
============================================
+ Coverage     78.25%   78.29%   +0.04%     
- Complexity     1947     1950       +3     
============================================
  Files           247      247              
  Lines          7910     7911       +1     
  Branches        747      747              
============================================
+ Hits           6190     6194       +4     
+ Misses         1301     1299       -2     
+ Partials        419      418       -1     
Impacted Files Coverage Δ Complexity Δ
...del/models/input/container/JointGridContainer.java 76.19% <0.00%> (+16.19%) 7.00% <0.00%> (+3.00%)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ec7d2f3...1eb680e. Read the comment docs.

@avgprog
Copy link
Contributor Author

avgprog commented Feb 18, 2021

Since, the code coverage test is failing, I suppose I need to add some test cases to check the newly added changes.

@avgprog avgprog self-assigned this Feb 22, 2021
@avgprog avgprog marked this pull request as ready for review February 23, 2021 07:49
@avgprog avgprog requested a review from ckittl February 23, 2021 07:49
Copy link
Member

@ckittl ckittl left a comment

Choose a reason for hiding this comment

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

Your test actually does only add limited value. Ideally, you would also test, that the log call has been made. However, in our case it is not easy to do.

It would work, if you would have the logger non-static and non-final. Then you could replace it with a mock or a spy on a delegate. However, completeness of the test doesn't justify design changes with given drawbacks.

@ckittl ckittl merged commit dbacf9e into dev Feb 25, 2021
@ckittl ckittl deleted the sb/#148-replacingInvalidGridException branch February 25, 2021 12:57
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.

Replace the InvalidGridException in JointGrid:50 with a logged warning
2 participants