Skip to content

#186: Refactor all string-based statistics report message assertions …#216

Merged
heshamMassoud merged 29 commits intomasterfrom
186-improve-assertions
Dec 22, 2017
Merged

#186: Refactor all string-based statistics report message assertions …#216
heshamMassoud merged 29 commits intomasterfrom
186-improve-assertions

Conversation

@heshamMassoud
Copy link
Copy Markdown
Contributor

@heshamMassoud heshamMassoud commented Dec 16, 2017

  1. Refactor all string-based statistics report message assertions to statistics counter assertions.
  2. Remove unneeded utils.
  3. Expose categorySyncStatistics#getNumberOfCategoriesWithMissingParents and make it instance method.
  4. Change of wording in the product sync statistics report message.
  5. Removal of some redundant unit tests.

Related Issues

#186

Todo

  • Add release notes entry.

@codecov-io
Copy link
Copy Markdown

codecov-io commented Dec 16, 2017

Codecov Report

Merging #216 into master will not change coverage.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff            @@
##             master     #216   +/-   ##
=========================================
  Coverage     93.06%   93.06%           
  Complexity      805      805           
=========================================
  Files            68       68           
  Lines          2077     2077           
  Branches        118      118           
=========================================
  Hits           1933     1933           
  Misses          123      123           
  Partials         21       21
Impacted Files Coverage Δ Complexity Δ
...s/sync/products/helpers/ProductSyncStatistics.java 100% <ø> (ø) 2 <0> (ø) ⬇️
...om/commercetools/sync/categories/CategorySync.java 99% <100%> (ø) 71 <3> (ø) ⬇️
...ync/categories/helpers/CategorySyncStatistics.java 100% <100%> (ø) 5 <1> (ø) ⬇️

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 ac9e5f6...bb4870c. Read the comment docs.

assertThat(syncStatistics.getProcessed()).isEqualTo(5);
assertThat(syncStatistics.getCreated()).isEqualTo(4);
assertThat(syncStatistics.getUpdated()).isEqualTo(1);
assertThat(syncStatistics.getFailed()).isEqualTo(0);
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.

having so long similar assertThat() chains i'd make some util assertSyncStatistic() method, which accepts a statistic instance and a list of expected values.

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.

@heshamMassoud heshamMassoud merged commit 47c3a28 into master Dec 22, 2017
@heshamMassoud heshamMassoud deleted the 186-improve-assertions branch December 22, 2017 12:43
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.

4 participants