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

refactor: replace some usages of assertTrue #957

Merged
merged 2 commits into from Oct 20, 2017

Conversation

Projects
None yet
3 participants
@AlexElin
Contributor

AlexElin commented Sep 25, 2017

replace assertTrue with more appropriate asserts

refactor: replace some usages of assertTrue
replace assertTrue with more appropriate asserts
@codecov-io

This comment has been minimized.

codecov-io commented Sep 25, 2017

Codecov Report

Merging #957 into master will increase coverage by 0.03%.
The diff coverage is n/a.

@@             Coverage Diff              @@
##             master     #957      +/-   ##
============================================
+ Coverage     65.88%   65.92%   +0.03%     
- Complexity     3556     3561       +5     
============================================
  Files           166      166              
  Lines         15223    15238      +15     
  Branches       2458     2464       +6     
============================================
+ Hits          10030    10045      +15     
+ Misses         4024     4022       -2     
- Partials       1169     1171       +2
@vlsi

Looks great.
Do you intend to add more changes here?

PS. Was BigDecimal compareTo->equals change intentional?

@@ -217,11 +218,11 @@ public void testGetObjectDecimal() throws Throwable {
cstmt.registerOutParameter(3, Types.DECIMAL);
cstmt.executeUpdate();
BigDecimal val = (BigDecimal) cstmt.getObject(1);
assertTrue(val.compareTo(new BigDecimal("999999999999999.000000000000000")) == 0);
assertEquals(new BigDecimal("999999999999999.000000000000000"), val);

This comment has been minimized.

@vlsi

vlsi Sep 26, 2017

Member

Do you know there's a difference between BigDecimal#equals and BigDecimal#compareTo==0?
The former treats 2.0 not equal to 2.00 while compareTo treats them as equal.

I'm not sure what was the intention here, however this looks like the most suspicious change.

This comment has been minimized.

@AlexElin

AlexElin Sep 27, 2017

Contributor

I didn't know.
I've reverted usage of eauals

refactor: replace some usages of assertTrue
revert some changes for testGetObjectDecimal
@AlexElin

This comment has been minimized.

Contributor

AlexElin commented Sep 27, 2017

I don't intend to add more changes here

@vlsi vlsi added this to the 42.1.5 milestone Oct 20, 2017

@vlsi vlsi merged commit c759a58 into pgjdbc:master Oct 20, 2017

2 checks passed

codecov/project 65.92% (+0.03%) compared to ee6443d
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@AlexElin AlexElin deleted the AlexElin:assertTrue branch Nov 3, 2017

@vlsi vlsi modified the milestones: 42.1.5, 42.2.0 Jan 8, 2018

rhavermans added a commit to bolcom/pgjdbc that referenced this pull request Jul 13, 2018

rhavermans added a commit to bolcom/pgjdbc that referenced this pull request Jul 13, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment