Skip to content

Conversation

bncbhm
Copy link
Contributor

@bncbhm bncbhm commented Oct 20, 2021

A first test for ForensicsTableModel, which uses the constructor to create an instance and check getId(), getColumns() and getRows().

@codecov
Copy link

codecov bot commented Oct 26, 2021

Codecov Report

Merging #341 (67d88a0) into master (237fba3) will increase coverage by 2.83%.
The diff coverage is n/a.

Impacted file tree graph

@@             Coverage Diff              @@
##             master     #341      +/-   ##
============================================
+ Coverage     50.04%   52.88%   +2.83%     
- Complexity      253      257       +4     
============================================
  Files            42       42              
  Lines          1093     1093              
  Branches         92       92              
============================================
+ Hits            547      578      +31     
+ Misses          510      479      -31     
  Partials         36       36              
Impacted Files Coverage Δ
.../jenkins/plugins/forensics/miner/SizePieChart.java 0.00% <0.00%> (ø)
...s/plugins/forensics/miner/ForensicsTableModel.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 237fba3...67d88a0. Read the comment docs.

Copy link
Member

@uhafner uhafner left a comment

Choose a reason for hiding this comment

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

AssertJ has some nice shortcuts to keep the code clean (and small)!

private static final int ONE_DAY = 60 * 60 * 24;

@Test
void shouldCreateForensicsTableModel(){
Copy link
Member

Choose a reason for hiding this comment

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

You should install the CheckStyle plugin, then those warnings will be flagged in IntelliJ.

Comment on lines 21 to 37
@Test
void shouldCreateForensicsTableModel(){
RepositoryStatistics statistics = new RepositoryStatistics();
ForensicsTableModel tableModel = new ForensicsTableModel(statistics);
assertThat(tableModel).isNotNull();
}


@Test
void getId() {
RepositoryStatistics statistics = new RepositoryStatistics();
ForensicsTableModel tableModel = new ForensicsTableModel(statistics);
assertThat(tableModel.getId()).isEqualTo(ForensicsJobAction.FORENSICS_ID);
}

@Test
void getColumns() {
Copy link
Member

Choose a reason for hiding this comment

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

I think it has no extra value if you separate this test in 3 methods: combine them in a single one.

void getId() {
RepositoryStatistics statistics = new RepositoryStatistics();
ForensicsTableModel tableModel = new ForensicsTableModel(statistics);
assertThat(tableModel.getId()).isEqualTo(ForensicsJobAction.FORENSICS_ID);
Copy link
Member

Choose a reason for hiding this comment

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

Use the generated assertions:

Suggested change
assertThat(tableModel.getId()).isEqualTo(ForensicsJobAction.FORENSICS_ID);
assertThat(tableModel).hasId(ForensicsJobAction.FORENSICS_ID);

Comment on lines 41 to 48
assertThat(tableModel.getColumns().size()).isEqualTo(7);
assertThat(tableModel.getColumns().get(0).getHeaderLabel()).isEqualTo(Messages.Table_Column_File());
assertThat(tableModel.getColumns().get(1).getHeaderLabel()).isEqualTo(Messages.Table_Column_AuthorsSize());
assertThat(tableModel.getColumns().get(2).getHeaderLabel()).isEqualTo(Messages.Table_Column_CommitsSize());
assertThat(tableModel.getColumns().get(3).getHeaderLabel()).isEqualTo(Messages.Table_Column_LastCommit());
assertThat(tableModel.getColumns().get(4).getHeaderLabel()).isEqualTo(Messages.Table_Column_AddedAt());
assertThat(tableModel.getColumns().get(5).getHeaderLabel()).isEqualTo(Messages.Table_Column_LOC());
assertThat(tableModel.getColumns().get(6).getHeaderLabel()).isEqualTo(Messages.Table_Column_Churn());
Copy link
Member

Choose a reason for hiding this comment

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

This would be much simpler without duplications:

        assertThat(tableModel.getColumns())
                .hasSize(7)
                .extracting(TableColumn::getHeaderLabel)
                .containsExactly(
                        Messages.Table_Column_File(), ... );

ForensicsTableModel tableModel = new ForensicsTableModel(statistics);
tableModel.getRows();
assertThat(tableModel.getRows().size()).isEqualTo(1);
}
Copy link
Member

Choose a reason for hiding this comment

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

Check one row content as well.

@bncbhm bncbhm closed this Oct 27, 2021
@bncbhm bncbhm reopened this Oct 27, 2021
@bncbhm bncbhm requested a review from uhafner October 27, 2021 15:38
@bncbhm bncbhm closed this Oct 27, 2021
@bncbhm bncbhm reopened this Oct 27, 2021
Comment on lines +44 to +45
assertThat(actual).isInstanceOf(ForensicsRow.class);
assertThat((ForensicsRow) actual).hasAuthorsSize(0);
Copy link
Member

Choose a reason for hiding this comment

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

A simpler way to avoid the cast is:

assertThat(actual).isInstanceOfSatisfying(ForensicsRow.class,
                row -> assertThat(row).hasAuthorsSize(0));


assertThat(forensicsRow)
.hasFileName(
"<a href=\"fileName.-734768633\" data-bs-toggle=\"tooltip\" data-bs-placement=\"left\" title=\"filename\">filename</a>")
Copy link
Member

Choose a reason for hiding this comment

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

This might be too fragile but is ok for now. (It might break just because the UI changes.)

@uhafner uhafner merged commit 87dca51 into jenkinsci:master Oct 27, 2021
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.

2 participants