Skip to content

Conversation

NikolasFunction
Copy link
Contributor

@NikolasFunction NikolasFunction commented Oct 28, 2021

Added Test for class RelativeCountTrendChart.java
JENKINS-66866

  • Make sure you are opening from a topic/feature/bugfix branch (right side) and not your main branch!
  • Ensure that the pull request title represents the desired changelog entry
  • Please describe what you did
  • Link to relevant issues in GitHub or Jira
  • Link to relevant pull requests, esp. upstream and downstream changes
  • Ensure you have provided tests - that demonstrates feature works or fixes the issue

@uhafner uhafner changed the title Added RelativeCountTrendChartTest Add test for RelativeCountTrendChartTest Nov 2, 2021
@uhafner uhafner added the tests Enhancement of tests label Nov 2, 2021
@codecov
Copy link

codecov bot commented Nov 2, 2021

Codecov Report

Merging #355 (404805e) into master (87dca51) will increase coverage by 4.11%.
The diff coverage is n/a.

Impacted file tree graph

@@             Coverage Diff              @@
##             master     #355      +/-   ##
============================================
+ Coverage     54.07%   58.18%   +4.11%     
- Complexity      263      280      +17     
============================================
  Files            42       42              
  Lines          1093     1093              
  Branches         92       92              
============================================
+ Hits            591      636      +45     
+ Misses          465      420      -45     
  Partials         37       37              
Impacted Files Coverage Δ
...ics/miner/RelativeCountForensicsSeriesBuilder.java 50.00% <0.00%> (+50.00%) ⬆️
.../plugins/forensics/miner/CodeMetricTrendChart.java 100.00% <0.00%> (+100.00%) ⬆️
...ugins/forensics/miner/CodeMetricSeriesBuilder.java 100.00% <0.00%> (+100.00%) ⬆️
...ugins/forensics/miner/RelativeCountTrendChart.java 100.00% <0.00%> (+100.00%) ⬆️
...er/RelativeCountCommitStatisticsSeriesBuilder.java 100.00% <0.00%> (+100.00%) ⬆️
...AddedVersusDeletedLinesForensicsSeriesBuilder.java 100.00% <0.00%> (+100.00%) ⬆️
...rsusDeletedLinesCommitStatisticsSeriesBuilder.java 100.00% <0.00%> (+100.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 87dca51...404805e. Read the comment docs.

* Tests RelativeCountTrendChart.
*
* @author Nikolas Paripovic
*
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
*

*
*/
class RelativeCountTrendChartTest {

Copy link
Member

Choose a reason for hiding this comment

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

Use new lines only to structure code

Suggested change


@Test
void shouldCreate() {

Copy link
Member

Choose a reason for hiding this comment

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

Suggested change

*/
class RelativeCountTrendChartTest {

private final RelativeCountTrendChart relativeCountTrendChart = new RelativeCountTrendChart();
Copy link
Member

Choose a reason for hiding this comment

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

No fields in unit tests, inline that into the method

@Test
void shouldCreate() {

Iterable<BuildResult<CommitStatisticsBuildAction>> buildResult = createBuildResults(false);
Copy link
Member

Choose a reason for hiding this comment

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

Inline that call to make it readable:

Suggested change
Iterable<BuildResult<CommitStatisticsBuildAction>> buildResult = createBuildResults(false);
Iterable<BuildResult<CommitStatisticsBuildAction>> buildResult = new ArrayList<>();


LinesChartModel linesChartModel = relativeCountTrendChart.create(buildResult, chartModelConfiguration, seriesBuilder);
assertThat(linesChartModel.getSeries()).hasSize(3);
assertThat(linesChartModel.getSeries()).allSatisfy(series -> assertThat(series.getData()).hasSize(4));
Copy link
Member

Choose a reason for hiding this comment

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

Optional: if you want to make sure that the chart actually is correct, you can use JSON assertions, see review in #351

@Fiesepille
Copy link

Seems to be a bug that commit/build #73bcd5d failed.
Following build #404805e succeeded, allthough the code styling was the only change.

@uhafner
Copy link
Member

uhafner commented Nov 3, 2021

Seems to be a bug that commit/build #73bcd5d failed.
Following build #404805e succeeded, allthough the code styling was the only change.

I think the download of an artifact from the maven repository was broken but that should be fixed now.

@uhafner uhafner merged commit ec99858 into jenkinsci:master Nov 3, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
tests Enhancement of tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants