You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@fineract.apache.org by GitBox <gi...@apache.org> on 2020/10/19 20:29:56 UTC
[GitHub] [fineract] vorburger opened a new pull request #1444: Introduce first "pure Spring" integration test, without Cargo (FINERACT-764)
vorburger opened a new pull request #1444:
URL: https://github.com/apache/fineract/pull/1444
The new SpringBootLoginTest is an illustration of what we could do more of.
NOTE: Use only https://:8443 instead of http://:7070 (redirects don't work)
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [fineract] awasum commented on a change in pull request #1444: Introduce first "pure Spring" integration test, without Cargo (FINERACT-764)
Posted by GitBox <gi...@apache.org>.
awasum commented on a change in pull request #1444:
URL: https://github.com/apache/fineract/pull/1444#discussion_r511819200
##########
File path: fineract-provider/src/test/java/org/apache/fineract/common/RestAssuredFixture.java
##########
@@ -34,14 +34,12 @@
private final int httpPort;
- private ResponseSpecification responseSpec;
- private RequestSpecification requestSpec;
+ private final ResponseSpecification responseSpec;
+ private final RequestSpecification requestSpec;
- public RestAssuredFixture(int http) {
-
- this.httpPort = http;
+ public RestAssuredFixture() {
+ this.httpPort = 844;
Review comment:
@vorburger Just curious and catching up to all the brilliant development done the past few weeks. Is this suppose to be port: 8443? or something new which I dont understand?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [fineract] xurror commented on pull request #1444: Introduce first "pure Spring" integration test, without Cargo (FINERACT-764)
Posted by GitBox <gi...@apache.org>.
xurror commented on pull request #1444:
URL: https://github.com/apache/fineract/pull/1444#issuecomment-724513147
Also, I noticed `./gradlew integrationTest` is no longer a thing and the readme was not updated to that regards, I don't know since when that task was removed, I just think you might want to update the readme on how to run tests now.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [fineract] vorburger commented on pull request #1444: Introduce first "pure Spring" integration test, without Cargo (FINERACT-764)
Posted by GitBox <gi...@apache.org>.
vorburger commented on pull request #1444:
URL: https://github.com/apache/fineract/pull/1444#issuecomment-717039700
@ptuomola @vidakovic I'm undecided / unclear where we should take this to.. thoughts? I guess we could... EITHER (probably best AFTER #1465 will have gone in):
a) move towards entirely ditching the Cargo plugin and not start the WAR but just use this new style to fire up Spring "in-process" for (all) ITs
b) forget about this approach and abandon this PR and just close FINERACT-764 (I don't actually really mind either)
c) have some mixed form - keep testing the WAR with the Cargo plugin for some basic tests, but move other ITs to this style.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [fineract] vorburger commented on a change in pull request #1444: Introduce first "pure Spring" integration test, without Cargo (FINERACT-764)
Posted by GitBox <gi...@apache.org>.
vorburger commented on a change in pull request #1444:
URL: https://github.com/apache/fineract/pull/1444#discussion_r512457801
##########
File path: fineract-provider/src/test/java/org/apache/fineract/common/RestAssuredFixture.java
##########
@@ -34,14 +34,12 @@
private final int httpPort;
- private ResponseSpecification responseSpec;
- private RequestSpecification requestSpec;
+ private final ResponseSpecification responseSpec;
+ private final RequestSpecification requestSpec;
- public RestAssuredFixture(int http) {
-
- this.httpPort = http;
+ public RestAssuredFixture() {
+ this.httpPort = 844;
Review comment:
@awasum well spotted, yes this looks wrong. It's surprising that it still worked - I think this line can probably be removed. I could do that, but I think there is a larger question of what direction we want to take re ITs - I'll comment on the main comment instead of inline here about that.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [fineract] vorburger commented on pull request #1444: Introduce first "pure Spring" integration test, without Cargo (FINERACT-764)
Posted by GitBox <gi...@apache.org>.
vorburger commented on pull request #1444:
URL: https://github.com/apache/fineract/pull/1444#issuecomment-726369896
> Also, I noticed `./gradlew integrationTest` is no longer a thing and the readme was not updated to that regards, I don't know since when that task was removed, I just think you might want to update the readme on how to run tests now.
That's since #1465; @vidakovic do you want to fix up the README? @xurror you could just raise a PR for it yourself as well? ;-)
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [fineract] github-actions[bot] closed pull request #1444: Introduce first "pure Spring" integration test, without Cargo (FINERACT-764)
Posted by GitBox <gi...@apache.org>.
github-actions[bot] closed pull request #1444:
URL: https://github.com/apache/fineract/pull/1444
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [fineract] xurror commented on pull request #1444: Introduce first "pure Spring" integration test, without Cargo (FINERACT-764)
Posted by GitBox <gi...@apache.org>.
xurror commented on pull request #1444:
URL: https://github.com/apache/fineract/pull/1444#issuecomment-724512816
Also, I noticed `./gradlew integrationTest` is no longer a thing, I don't know since when that task was removed, I just think you might want to update the readme on how to run tests now.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [fineract] vidakovic commented on pull request #1444: Introduce first "pure Spring" integration test, without Cargo (FINERACT-764)
Posted by GitBox <gi...@apache.org>.
vidakovic commented on pull request #1444:
URL: https://github.com/apache/fineract/pull/1444#issuecomment-717137108
> @ptuomola @vidakovic I'm undecided / unclear where we should take this to.. thoughts? I guess we could... EITHER (probably best AFTER #1465 will have gone in):
>
> a) move towards entirely ditching the Cargo plugin and not start the WAR but just use this new style to fire up Spring "in-process" for (all) ITs
a) I'd prefer to go that way. The Cargo plugin requires the WAR artifact and is in the end an external process... just to say: a lot of moving parts to take care of. During a normal build it maybe doesn't make much of a difference for devs, but if you execute e. g. "./gradlew :integration-tests:test" then Cargo doesn't shutdown for some reason and leaves a zombie process (just found that out yesterday). And I have a hunch that Spring tests would run faster.
> b) forget about this approach and abandon this PR and just close [FINERACT-764](https://issues.apache.org/jira/browse/FINERACT-764) (I don't actually really mind either)
>
> c) have some mixed form - keep testing the WAR with the Cargo plugin for some basic tests, but move other ITs to this style.
c) Realistically, we will probably take a moment or two to refactor all integration tests. So for a while we would be probably running in a "hybrid" mode. Maybe we could do this migration together with moving towards Fineract Client usage.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [fineract] xurror commented on pull request #1444: Introduce first "pure Spring" integration test, without Cargo (FINERACT-764)
Posted by GitBox <gi...@apache.org>.
xurror commented on pull request #1444:
URL: https://github.com/apache/fineract/pull/1444#issuecomment-718050176
I support ditching cargo, we could save us a lot of time running tests.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [fineract] vorburger commented on pull request #1444: Introduce first "pure Spring" integration test, without Cargo (FINERACT-764)
Posted by GitBox <gi...@apache.org>.
vorburger commented on pull request #1444:
URL: https://github.com/apache/fineract/pull/1444#issuecomment-717040235
BTW #1422 has some related thoughts re. our testing strategy, I'll comment over there as well.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [fineract] xurror removed a comment on pull request #1444: Introduce first "pure Spring" integration test, without Cargo (FINERACT-764)
Posted by GitBox <gi...@apache.org>.
xurror removed a comment on pull request #1444:
URL: https://github.com/apache/fineract/pull/1444#issuecomment-724512816
Also, I noticed `./gradlew integrationTest` is no longer a thing, I don't know since when that task was removed, I just think you might want to update the readme on how to run tests now.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [fineract] github-actions[bot] commented on pull request #1444: Introduce first "pure Spring" integration test, without Cargo (FINERACT-764)
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #1444:
URL: https://github.com/apache/fineract/pull/1444#issuecomment-743927137
This pull request seems to be stale. Are you still planning to work on it? We will automatically close it in 30 days.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [fineract] vorburger commented on pull request #1444: Introduce first "pure Spring" integration test, without Cargo (FINERACT-764)
Posted by GitBox <gi...@apache.org>.
vorburger commented on pull request #1444:
URL: https://github.com/apache/fineract/pull/1444#issuecomment-712424701
@ptuomola @vidakovic how do you like this? Where could & should we take this to next?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org