Skip to content
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

Clean up the build and fix some tests #272

Merged
merged 4 commits into from
Sep 10, 2017
Merged

Conversation

johnynek
Copy link
Collaborator

@johnynek johnynek commented Sep 9, 2017

We migrated the tests long ago from scalacheck driven to scalatest driven.

We missed some of the correct conversion so some of the tests are no-op.

Side note, I pretty much hate how flexible scalatest is. It is very difficult to see if something is a no-op. This have been disabled for years.

Probably more are also but I didn't run them down yet.

@johnynek
Copy link
Collaborator Author

johnynek commented Sep 9, 2017

@piyushnarang can you review this?

@codecov-io
Copy link

codecov-io commented Sep 9, 2017

Codecov Report

Merging #272 into develop will increase coverage by 5.51%.
The diff coverage is n/a.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop     #272      +/-   ##
===========================================
+ Coverage    66.14%   71.65%   +5.51%     
===========================================
  Files           50       50              
  Lines         1397     1397              
  Branches        50       50              
===========================================
+ Hits           924     1001      +77     
+ Misses         473      396      -77
Impacted Files Coverage Δ
...c/main/scala/com/twitter/bijection/Injection.scala 48.21% <0%> (+3.57%) ⬆️
...a/com/twitter/bijection/CollectionBijections.scala 46.57% <0%> (+6.84%) ⬆️
...a/com/twitter/bijection/CollectionInjections.scala 96.77% <0%> (+16.12%) ⬆️
...cala/com/twitter/bijection/NumericInjections.scala 85.13% <0%> (+27.02%) ⬆️
...cala/com/twitter/bijection/NumericBijections.scala 100% <0%> (+58.13%) ⬆️
.../scala/com/twitter/bijection/ModDivInjection.scala 100% <0%> (+76.92%) ⬆️

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 72af31f...d773b14. Read the comment docs.

@johnynek
Copy link
Collaborator Author

johnynek commented Sep 9, 2017

+5.5% coverage! woot!

@ianoc or @sriramkrishnan can you folks give me a +1 on this? Pretty safe: just fixes some broken tests and adds some more.

@johnynek johnynek merged commit 2a8aa29 into develop Sep 10, 2017
@ianoc
Copy link
Collaborator

ianoc commented Sep 11, 2017

very nice! +5.5% is a decent jump!

@piyushnarang
Copy link
Collaborator

👍

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.

5 participants