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

fix: package scram:client classes, so scram works when using a shaded jar #1091

Merged
merged 7 commits into from Jan 23, 2018

Conversation

Projects
None yet
3 participants
@davecramer
Copy link
Member

davecramer commented Jan 23, 2018

No description provided.

@codecov-io

This comment has been minimized.

Copy link

codecov-io commented Jan 23, 2018

Codecov Report

Merging #1091 into master will decrease coverage by 0.06%.
The diff coverage is n/a.

@@             Coverage Diff             @@
##             master   #1091      +/-   ##
===========================================
- Coverage     67.27%   67.2%   -0.07%     
+ Complexity     3666    3660       -6     
===========================================
  Files           170     170              
  Lines         15628   15628              
  Branches       2526    2526              
===========================================
- Hits          10513   10503      -10     
- Misses         3929    3934       +5     
- Partials       1186    1191       +5
@vlsi

This comment has been minimized.

Copy link
Member

vlsi commented Jan 23, 2018

Looks good, however it does mean the test is performed before shading.

What do you think of making TEST_CLIENT Travis job run through SCRAM?
It does use "shaded jar", so it should identify the problem: https://github.com/pgjdbc/pgjdbc/blob/master/.travis/travis_build.sh#L72

@davecramer

This comment has been minimized.

Copy link
Member

davecramer commented Jan 23, 2018

@vlsi

This comment has been minimized.

Copy link
Member

vlsi commented Jan 23, 2018

I was wondering how to do that.

Unfortunately, SCRAM is tested with HEAD backend only (see https://github.com/pgjdbc/pgjdbc/blob/master/.travis.yml#L32 )

So the solution would be to put the following lines under if PG_VERSION=HEAD condition:
https://github.com/pgjdbc/pgjdbc/blob/master/.travis/travis_build.sh#L77-L79

That is just another action after existing mvn test.

Would you please try that?

git clone --depth=10 https://github.com/clojure/java.jdbc.git
cd java.jdbc
TEST_DBS=postgres TEST_POSTGRES_USER=test TEST_POSTGRES_DBNAME=test mvn test -Djava.jdbc.test.pgjdbc.version=$PROJECT_VERSION
fi
fi

This comment has been minimized.

@vlsi

vlsi Jan 23, 2018

Member

Please move it out of if [[ "${TEST_CLIENTS}" == *"Y" ]];

davecramer and others added some commits Jan 23, 2018

@vlsi vlsi changed the title fix #1090 scram client missing fix: package scram:client classes, so scram works when using a shaded jar Jan 23, 2018

@vlsi vlsi added this to the 42.2.1 milestone Jan 23, 2018

@vlsi vlsi merged commit 1a89290 into pgjdbc:master Jan 23, 2018

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

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