Feat/#502 consul integrate test #503

jiachen1120 · 2019-05-06T20:40:52Z

Enhance the registry test by integrate with https://www.baeldung.com/docker-test-containers, so a real consul can be established automatically when testing.

Since this integrate test depends on docker so it should be disabled all the time unless it is used.

Related issue: #502

…eat/#502-consul-integrate-test

codecov-io · 2019-05-06T20:44:46Z

Codecov Report

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

@@             Coverage Diff              @@
##             master     #503      +/-   ##
============================================
- Coverage     49.67%   49.65%   -0.02%     
  Complexity     1829     1829              
============================================
  Files           258      258              
  Lines         11214    11214              
  Branches       1607     1607              
============================================
- Hits           5570     5568       -2     
- Misses         4985     4987       +2     
  Partials        659      659

Impacted Files	Coverage Δ	Complexity Δ
...ver/src/main/java/com/networknt/server/Server.java	`35.51% <0%> (-0.82%)`	`17% <0%> (ø)`

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 8c8a7fb...fdc91a1. Read the comment docs.

stevehu · 2019-05-06T21:22:04Z

@jiachen1120 This is a very good idea to test with a real container; however, we cannot expect all developers to use docker on their development environment. We have some repositories that depending on the external containers for testing and normally we create an extra folder called integration along with test for integration test. All unit tests will be in the test folder and they don't rely on the container and all integration test cases are in the integration folder and they rely on the container. The following is an example in light-session-4j and the integration test won't be invoked when mvn clean package is called.

https://github.com/networknt/light-session-4j/tree/master/redis-manager/src

ddobrin · 2019-05-06T21:48:59Z

@jiachen1120 : while I agree that it is a neat idea, please be aware that developers at my client are building/testing in several APIs on Windows machines, on prem, in order to have access to certain resources.

In those cases, they don't use Docker, as they don't have it on their machines.

jiachen1120 · 2019-05-07T14:34:09Z

@stevehu Cool, I will migrate this test to an integrate folder

jiachen1120 · 2019-05-07T14:37:53Z

@ddobrin Hey, after migrating this test to integration test folder. They would not be invoked during mvn build. They can be used just by developer to testing the framework.

stevehu · 2019-05-07T14:38:23Z

I looked at the test container project and it is very promising. It is much more convenient than our docker-compose. Good job!

DSchrupert · 2019-05-13T15:08:10Z

server/src/integration/java/com/networknt/server/ConsulRegistryIT.java

+ * This test is an automatic test for real consul registry and discovery by using testContainers.
+ * Since it depends on docker so it should be disabled all the time unless it is used.
+ */
+@Ignore


Is there a better way to enable/disable then (what I presume) is manually removing the @ Ignore flag? What is the process for running full integration tests?

We can enable the test case. If you don't want to execute the integration test cases, use mvn clean package. If you want to run all integration tests along with unit tests, then you can use mvn clean install.

Maybe we don't want to go that far in case someone wants to install the dependencies but doesn't have docker installed.. could we add a maven profile for running integration tests?

Using maven profile is another option. The key point is we need to ensure that broader users can compile without the docker installed and our developers can trigger integration tests with a switch. I really have no idea which commands most developers using mvn package or mvn install.

@stevehu Hey. I think mvn install would be more popular to use. So I updated the pom to disable the integration test execution when using mvn install.

In this way,mvn install only run the unit test.
And the following commands are using for executing integration tests:
mvn verify -DskipITs=false
mvn integration-tests -DskipITs=false
mvn clean install -DskipITs=false

What do you think?

I think it is a very good idea. mvn verify -DskipITs=false can execute integration tests when they are annotated as ignored?

I just tested. If @ignore annotated inside testing code. There seems no way to execute them. I think we can remove all the ignore annotation in the integration test if the default value for skipITs is true.

Got it. I like the idea. We need to make sure that everything is transparent to new developers.

…eat/#502-consul-integrate-test

DSchrupert · 2019-05-16T13:03:47Z

@jiachen1120 i'm getting failures when running the build locally.. is there something i'm missing?

[ERROR] Failures: 
[ERROR]   ConsulRegistryIT.testDiscoverCaching:194 expected:<...//192.168.85.121:495[90]> but was:<...//192.168.85.121:495[89]>
[ERROR]   ConsulRegistryIT.testDiscoverWithNonEmptyStringEnvTag:169 expected:<...//192.168.85.121:495[90]> but was:<...//192.168.85.121:495[89]>

…folder to the target folder -CURD some config files due to fix test cases (those were not configured correctly in previous versions)

-test configs will be generated either from integration folder or under test folder depends on the profile, cannot include both because file with same name will have only one copy under config directory in phase "generate-test-sources". -when run mvn clean install will as usual, when run mvn clean install -P integration-test will execute integration tests

…consul-integrate-test

DSchrupert · 2019-06-11T14:34:23Z

@jiachen1120 i'm getting failures when running the build locally.. is there something i'm missing?

[ERROR] Failures: 
[ERROR]   ConsulRegistryIT.testDiscoverCaching:194 expected:<...//192.168.85.121:495[90]> but was:<...//192.168.85.121:495[89]>
[ERROR]   ConsulRegistryIT.testDiscoverWithNonEmptyStringEnvTag:169 expected:<...//192.168.85.121:495[90]> but was:<...//192.168.85.121:495[89]>

This passes for me now with command mvn verify -P integration-test

cherry-pick 2638299 Feat/#502 consul integrate test (#503) * Fixed the scope caching error * Added dependence of container test into pom * Moved the test cases to server module * Enhanced the registry test with testContainer. * Migrate consul container test to integration folder * Reverted unnecessary change * Changed name * Disable the integration test for mvn clean install * Removed unused line * -now integration test will copy the config under its own integration folder to the target folder -CURD some config files due to fix test cases (those were not configured correctly in previous versions) * -move integration plugins to integration-test profile -test configs will be generated either from integration folder or under test folder depends on the profile, cannot include both because file with same name will have only one copy under config directory in phase "generate-test-sources". -when run mvn clean install will as usual, when run mvn clean install -P integration-test will execute integration tests * fixes the typo # Conflicts: # pom.xml

cherry-pick 26382993 Feat/#502 consul integrate test (#503) * Fixed the scope caching error * Added dependence of container test into pom * Moved the test cases to server module * Enhanced the registry test with testContainer. * Migrate consul container test to integration folder * Reverted unnecessary change * Changed name * Disable the integration test for mvn clean install * Removed unused line * -now integration test will copy the config under its own integration folder to the target folder -CURD some config files due to fix test cases (those were not configured correctly in previous versions) * -move integration plugins to integration-test profile -test configs will be generated either from integration folder or under test folder depends on the profile, cannot include both because file with same name will have only one copy under config directory in phase "generate-test-sources". -when run mvn clean install will as usual, when run mvn clean install -P integration-test will execute integration tests * fixes the typo # Conflicts: # pom.xml

* merged #503 from master to 1.6.x cherry-pick 2638299 Feat/#502 consul integrate test (#503) * Fixed the scope caching error * Added dependence of container test into pom * Moved the test cases to server module * Enhanced the registry test with testContainer. * Migrate consul container test to integration folder * Reverted unnecessary change * Changed name * Disable the integration test for mvn clean install * Removed unused line * -now integration test will copy the config under its own integration folder to the target folder -CURD some config files due to fix test cases (those were not configured correctly in previous versions) * -move integration plugins to integration-test profile -test configs will be generated either from integration folder or under test folder depends on the profile, cannot include both because file with same name will have only one copy under config directory in phase "generate-test-sources". -when run mvn clean install will as usual, when run mvn clean install -P integration-test will execute integration tests * fixes the typo # Conflicts: # pom.xml * Fix/#519 keystore fall back (#525) * Fixed the scope caching error * Fix keystore and truststore fall back * Fixed keystore and truststore fall back * Removed incorrect/unnecessary log message * Added comments * Created TlsUtil to support load keystore/truststore * Added test cases

* merged #503 from master to 1.6.x cherry-pick 2638299 Feat/#502 consul integrate test (#503) * Fixed the scope caching error * Added dependence of container test into pom * Moved the test cases to server module * Enhanced the registry test with testContainer. * Migrate consul container test to integration folder * Reverted unnecessary change * Changed name * Disable the integration test for mvn clean install * Removed unused line * -now integration test will copy the config under its own integration folder to the target folder -CURD some config files due to fix test cases (those were not configured correctly in previous versions) * -move integration plugins to integration-test profile -test configs will be generated either from integration folder or under test folder depends on the profile, cannot include both because file with same name will have only one copy under config directory in phase "generate-test-sources". -when run mvn clean install will as usual, when run mvn clean install -P integration-test will execute integration tests * fixes the typo # Conflicts: # pom.xml * Fix/#519 keystore fall back (#525) * Fixed the scope caching error * Fix keystore and truststore fall back * Fixed keystore and truststore fall back * Removed incorrect/unnecessary log message * Added comments * Created TlsUtil to support load keystore/truststore * Added test cases * - Made config module loading strategy pluggable - Added test cases * - Added comments - Added more test case * - Added more test cases * removed skip.unit.tests flag, move dup use of plugins to plugin management, try to use <skipTests> in integration profile * -revert changes, change skip.unit.tests to skipTests -based on https://issues.apache.org/jira/browse/MNG-4979, cannot override skipTests once it's set by skip.unit.tests * added missing itegration test config files, because now itegration test config will be totally sepreate from unit test configs * fixes #590 by adding client.truststore and client.keystore into server integration * - Fix consul integration test

* Fixed the scope caching error * Added dependence of container test into pom * Moved the test cases to server module * Enhanced the registry test with testContainer. * Migrate consul container test to integration folder * Reverted unnecessary change * Changed name * Disable the integration test for mvn clean install * Removed unused line * -now integration test will copy the config under its own integration folder to the target folder -CURD some config files due to fix test cases (those were not configured correctly in previous versions) * -move integration plugins to integration-test profile -test configs will be generated either from integration folder or under test folder depends on the profile, cannot include both because file with same name will have only one copy under config directory in phase "generate-test-sources". -when run mvn clean install will as usual, when run mvn clean install -P integration-test will execute integration tests * fixes the typo * update travis to jdk11

jiachen1120 added 7 commits April 18, 2019 18:25

Fixed the scope caching error

5dade37

Merge branch 'master' of https://github.com/networknt/light-4j

ab58c2c

Merge branch 'master' of https://github.com/networknt/light-4j

cff9763

Added dependence of container test into pom

7e76801

Moved the test cases to server module

52ef5f3

Enhanced the registry test with testContainer.

2ad53a5

Merge branch 'master' of https://github.com/networknt/light-4j into f…

27c6826

…eat/#502-consul-integrate-test

jiachen1120 requested review from stevehu and DSchrupert May 6, 2019 20:40

jiachen1120 added 3 commits May 7, 2019 10:50

Migrate consul container test to integration folder

a03335c

Reverted unnecessary change

913294c

Changed name

9a16b4f

DSchrupert reviewed May 13, 2019

View reviewed changes

jiachen1120 added 3 commits May 15, 2019 13:31

Disable the integration test for mvn clean install

fa5acf8

Merge branch 'master' of https://github.com/networknt/light-4j into f…

8640ba7

…eat/#502-consul-integrate-test

Removed unused line

411138a

DSchrupert added the dont-merge PR: Not ready to be merged label Jun 3, 2019

stevehu requested a review from ddobrin June 5, 2019 17:02

BalloonWen added 3 commits June 10, 2019 12:27

-now integration test will copy the config under its own integration …

1e70b4d

…folder to the target folder -CURD some config files due to fix test cases (those were not configured correctly in previous versions)

Merge branch 'master' of github.com:networknt/light-4j into feat/#502-…

fdc91a1

…consul-integrate-test

DSchrupert removed the dont-merge PR: Not ready to be merged label Jun 11, 2019

DSchrupert approved these changes Jun 11, 2019

View reviewed changes

merge from master

764828d

stevehu approved these changes Jun 14, 2019

View reviewed changes

stevehu added 2 commits June 13, 2019 22:02

fixes the typo

0a9d076

update travis to jdk11

0133011

stevehu merged commit 2638299 into master Jun 14, 2019

stevehu deleted the feat/#502-consul-integrate-test branch June 14, 2019 12:13

BalloonWen mentioned this pull request Jun 26, 2019

Fixes merge to 1.6.x #564

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feat/#502 consul integrate test #503

Feat/#502 consul integrate test #503

jiachen1120 commented May 6, 2019

codecov-io commented May 6, 2019 •

edited

Loading

stevehu commented May 6, 2019

ddobrin commented May 6, 2019

jiachen1120 commented May 7, 2019

jiachen1120 commented May 7, 2019

stevehu commented May 7, 2019

DSchrupert May 13, 2019

stevehu May 13, 2019

DSchrupert May 13, 2019

stevehu May 13, 2019

jiachen1120 May 15, 2019

stevehu May 15, 2019

jiachen1120 May 15, 2019

stevehu May 15, 2019

DSchrupert commented May 16, 2019

DSchrupert commented Jun 11, 2019

Feat/#502 consul integrate test #503

Feat/#502 consul integrate test #503

Conversation

jiachen1120 commented May 6, 2019

codecov-io commented May 6, 2019 • edited Loading

Codecov Report

stevehu commented May 6, 2019

ddobrin commented May 6, 2019

jiachen1120 commented May 7, 2019

jiachen1120 commented May 7, 2019

stevehu commented May 7, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

DSchrupert commented May 16, 2019

DSchrupert commented Jun 11, 2019

codecov-io commented May 6, 2019 •

edited

Loading