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 2021/10/18 14:37:17 UTC

[GitHub] [fineract] ankita10r opened a new pull request #1894: FINERACT-1410 :Allow Payment types to map to all types of Ledgers

ankita10r opened a new pull request #1894:
URL: https://github.com/apache/fineract/pull/1894

## Description

Describe the changes made and why they were made.

Ignore if these details are present on the associated [Apache Fineract JIRA ticket](https://github.com/apache/fineract/pull/1284).

## Checklist

Please make sure these boxes are checked before submitting your pull request - thanks!

- [ ] Write the commit message as per https://github.com/apache/fineract/#pull-requests

- [ ] Acknowledge that we will not review PRs that are not passing the build _("green")_ - it is your responsibility to get a proposed PR to pass the build, not primarily the project's maintainers.

- [ ] Create/update unit or integration tests for verifying the changes made.

- [ ] Follow coding conventions at https://cwiki.apache.org/confluence/display/FINERACT/Coding+Conventions.

- [ ] Add required Swagger annotation and update API documentation at fineract-provider/src/main/resources/static/api-docs/apiLive.htm with details of any API changes

- [ ] Submission is not a "code dump". (Large changes can be made "in repository" via a branch. Ask on the developer mailing list for guidance, if required.)

FYI our guidelines for code reviews are at https://cwiki.apache.org/confluence/display/FINERACT/Code+Review+Guide.

--
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.

To unsubscribe, e-mail: commits-unsubscribe@fineract.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [fineract] avikganguly01 commented on pull request #1894: FINERACT-1410 :Allow Payment types to map to all types of Ledgers

Posted by GitBox <gi...@apache.org>.

avikganguly01 commented on pull request #1894:
URL: https://github.com/apache/fineract/pull/1894#issuecomment-948901095

These is a partial list of flaky tests I could find today which doesn't pass consistently :-
- testApplyAnnualFeeForSavingsJobOutcome
- testApplyDueFeeChargesForSavingsJobOutcome
- testApplyHolidaysToLoansJobOutcome
- testApplyPenaltyForOverdueLoansJobOutcome
- testAvoidUnncessaryPenaltyWhenAmountZeroForOverdueLoansJobOutcome
- testJobExecution
- testLoanImport
- creditBureauConfigurationTest
- testSavingsImport

I don’t have steps to consistently fail those test cases and reproduce it. This will require deeper analysis of each test case. We should assign some of these tickets to their original authors and skip the less important tests till volunteers fix them.

> All the PRs were failing with the same error.
Thank you for spotting and highlighting this. This doesn't help reviewers prevent the problem.

> But the point is: it should be the responsibility of the person submitting the PR to ensure the build passes before it is submitted, not for us to prove that the failure is due to their PR.
In general, yes. In the current state of Fineract branch, I disagree. Right now, some authors I know keeps sending the PR after every hour till Travis passes the build and follows custom workflows to pass the build.

Example of one such flow :

1. Wait for Travis or gradle test task to finish ( Check after 30-60 minutes - not receiving notifications)
2. If travis build failed, identify the test case that failed from travis log.
3a. If the test case clearly appears to be broken by the current changes, fix it and resend PR. [Subjective Step]
3b. If the test case isn't closely related to the PR, run Travis on the PR again to see if Travis results are idempotent.
4. If the test case appears to be unrelated to the current change, make sure of this by examining and running this test case locally.
5a. Run this test case on a fresh ‘develop’ branch to check if this fails, this is a proof that this test case is broken and it is not due to current changes in the PR.
5b. If the test case doesn't fail on develop, then code fix is required.

---- Configuration Guidelines
If your PR includes a Global Configuration, update the teardown related code at : -
Update the verifyAllDefaultGlobalConfigurations() in GlobalConfigurationHelper.
//There are currently 27 global configurations.
Assert.assertEquals(27, expectedGlobalConfigurations.size());
Assert.assertEquals(27, actualGlobalConfigurations.size());
Also add default configurations value to the list for teardown in getAllDefaultGlobalConfigurations() in GlobalConfigurationHelper.java

-----
Better chances of getting a green build -
- Run integration test locally on a fresh tenant (time-wise expensive) everytime
- If a newly written integration test affects any other integration test, then there should be a teardown function written to restore to initial state so that other test cases execute on default state.

The build is failing again without any clear reason : -
https://app.travis-ci.com/github/apache/fineract/builds/240299730

--
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.

To unsubscribe, e-mail: commits-unsubscribe@fineract.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [fineract] ptuomola commented on pull request #1894: FINERACT-1410 :Allow Payment types to map to all types of Ledgers

Posted by GitBox <gi...@apache.org>.

ptuomola commented on pull request #1894:
URL: https://github.com/apache/fineract/pull/1894#issuecomment-951995931


   @avikganguly01 I've just looked through the last couple of dozen or so failed builds, and did not see any of the ones on this list. Perhaps you could flag when you next see one of these failing, and we can then investigate & fix the root cause.


-- 
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.

To unsubscribe, e-mail: commits-unsubscribe@fineract.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [fineract] ptuomola commented on pull request #1894: FINERACT-1410 :Allow Payment types to map to all types of Ledgers

Posted by GitBox <gi...@apache.org>.

ptuomola commented on pull request #1894:
URL: https://github.com/apache/fineract/pull/1894#issuecomment-947597093


   All the PRs were failing with the same error. As soon as I reverted this,
   they started passing. To me it seems clear that this PR caused them to
   fail.
   
   And yes, I do think it should be the responsibility of the person
   submitting the PR to ensure that the tests pass. That’s what our PR
   guidelines say as well, and that’s certainly the standard that I’ve always
   followed with my own PRs.
   
   On Wed, 20 Oct 2021 at 7:49 PM, Avik Ganguly ***@***.***>
   wrote:
   
   > @avikganguly01 <https://github.com/avikganguly01> @francisguchie
   > <https://github.com/francisguchie> @ankita10r
   > <https://github.com/ankita10r> @IOhacker <https://github.com/IOhacker>
   >
   > This has now broken the build, and all the other builds are failing. We
   > should *never* merge failing builds please. I will try to revert this
   > change - please can you fix the failing tests before merging again.
   >
   > @IOhacker <https://github.com/IOhacker> : There are 10+ tests like
   > testSkippingRepaymentOnFirstDayOfMonth() which fails randomly and has
   > nothing to do with this PR.
   >
   > @ptuomola <https://github.com/ptuomola> : Can you be more specific with
   > regards to which integration test failed because of this PR? Do not revert
   > this change unless proven otherwise. The author of this PR is not
   > responsible for fixing all the integration tests which fail due to no fault
   > of hers.
   >
   > —
   > You are receiving this because you were mentioned.
   > Reply to this email directly, view it on GitHub
   > <https://github.com/apache/fineract/pull/1894#issuecomment-947587901>, or
   > unsubscribe
   > <https://github.com/notifications/unsubscribe-auth/AASJVCRDMVAGVHTM4T7YCPTUH2UDRANCNFSM5GG2FFOQ>
   > .
   > Triage notifications on the go with GitHub Mobile for iOS
   > <https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675>
   > or Android
   > <https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>.
   >
   >
   


-- 
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.

To unsubscribe, e-mail: commits-unsubscribe@fineract.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [fineract] fintecheando commented on pull request #1894: FINERACT-1410 :Allow Payment types to map to all types of Ledgers

Posted by GitBox <gi...@apache.org>.

fintecheando commented on pull request #1894:
URL: https://github.com/apache/fineract/pull/1894#issuecomment-950169044


   Hello, I have added for SonarQube a Wait time, if required I will add a wait_time for the building.


-- 
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.

To unsubscribe, e-mail: commits-unsubscribe@fineract.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [fineract] ptuomola commented on pull request #1894: FINERACT-1410 :Allow Payment types to map to all types of Ledgers

Posted by GitBox <gi...@apache.org>.

ptuomola commented on pull request #1894:
URL: https://github.com/apache/fineract/pull/1894#issuecomment-947608321


   And yes, if you look at the code and the failing test, it's obvious that this PR broke the test. 
   
   But the point is: it should be the responsibility of the person submitting the PR to ensure the build passes before it is submitted, not for us to prove that the failure is due to their PR. 


-- 
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.

To unsubscribe, e-mail: commits-unsubscribe@fineract.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [fineract] avikganguly01 commented on pull request #1894: FINERACT-1410 :Allow Payment types to map to all types of Ledgers

Posted by GitBox <gi...@apache.org>.

avikganguly01 commented on pull request #1894:
URL: https://github.com/apache/fineract/pull/1894#issuecomment-951978445


   @ptuomola @fynmanoj @IOhacker :
   
   This is the approximate list of flaky tests which new contributors have to pass to get a green build : -
   
   1. ClientSavingsIntegrationTest. testSavingsAccount_DormancyTracking()
   2. FinancialActivityAccountsTest. testFinancialActivityAccounts()
   3.GlobalConfigInterestChargedFromDateSameAsDisbursalDateTest.testInterestChargedFromDateSameAsDisbursalDate()
   4. GlobalConfigurationTest. testGlobalConfigForcePasswordResetDays()
   5. GlobalConfigurationTest. testGlobalConfigurationIsCacheEnabled()
   6. GlobalConfigurationTest. testGlobalConfigurations()
   7. creditBureauConfigurationTest
   8. LoanReschedulingWithinCenterTest. testCenterReschedulingLoansWithInterestRecalculationEnabled()
   9. LoanReschedulingWithinCenterTest. testCenterReschedulingMultiTrancheLoansWithInterestRecalculationEnabled()
   10. SavingsAccountsTest. activateSavingsAccount()
   11. SavingsAccountsTest. approveSavingsAccount()
   12. SavingsAccountsTest. submitSavingsAccountsApplication()
   13. SchedulerJobsTest. testDateFormat()
   14. SchedulerJobsTest. testFlippingJobsActiveStatus()
   15. SchedulerJobsTest. testNumberOfJobs()
   16. SchedulerJobsTest. testTriggeringManualExecutionOfAllSchedulerJobs()
   17. SchedulerJobsTestResults. testApplyAnnualFeeForSavingsJobOutcome()
   18. SchedulerJobsTestResults. testApplyDueFeeChargesForSavingsJobOutcome()
   19. SchedulerJobsTestResults. testApplyHolidaysToLoansJobOutcome()
   20. SchedulerJobsTestResults. testApplyPenaltyForOverdueLoansJobOutcome()
   21. SchedulerJobsTestResults. testAvoidUnncessaryPenaltyWhenAmountZeroForOverdueLoansJobOutcome()
   22. SchedulerJobsTestResults. testExecuteStandingInstructionsJobOutcome()
   23. SchedulerJobsTestResults. testInterestPostingForSavingsJobOutcome()
   24. SchedulerJobsTestResults. testInterestTransferForSavings()
   25. SchedulerJobsTestResults. testTransferFeeForLoansFromSavingsJobOutcome()
   26. SchedulerJobsTestResults. testUpdateAccountingRunningBalancesJobOutcome()
   27. SchedulerJobsTestResults. testUpdateLoanArrearsAgingJobOutcome()
   28. SchedulerJobsTestResults. testUpdateLoanPaidInAdvanceJobOutcome()
   29. SchedulerJobsTestResults. testUpdateOverdueDaysForNPA()


-- 
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.

To unsubscribe, e-mail: commits-unsubscribe@fineract.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [fineract] avikganguly01 edited a comment on pull request #1894: FINERACT-1410 :Allow Payment types to map to all types of Ledgers

Posted by GitBox <gi...@apache.org>.

avikganguly01 edited a comment on pull request #1894:
URL: https://github.com/apache/fineract/pull/1894#issuecomment-950615591


   @fintecheando (cc @ptuomola @awasum) : Thank you for adding SonarQube! Do you have any inputs for reducing test time? Even in some M1 chips, build & test commands used by Travis is taking 3 hours plus. 


-- 
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.

To unsubscribe, e-mail: commits-unsubscribe@fineract.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [fineract] avikganguly01 edited a comment on pull request #1894: FINERACT-1410 :Allow Payment types to map to all types of Ledgers

Posted by GitBox <gi...@apache.org>.

avikganguly01 edited a comment on pull request #1894:
URL: https://github.com/apache/fineract/pull/1894#issuecomment-948901095

@awasum @fynmanoj @ptuomola

> All the PRs were failing with the same error.

Thank you for spotting and highlighting this. This doesn't help authors or reviewers from preventing the problem.

> But the point is: it should be the responsibility of the person submitting the PR to ensure the build passes before it is submitted, not for us to prove that the failure is due to their PR.

In general, yes. In the current state of Fineract branch, I disagree. Right now, some authors I know keeps sending the PR after every hour till Travis passes the build and follows custom workflows to pass the build.

Example of one such tedious flow :

The build is failing again without any clear reason : -
https://app.travis-ci.com/github/apache/fineract/builds/240299730

--
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.

To unsubscribe, e-mail: commits-unsubscribe@fineract.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [fineract] francisguchie commented on pull request #1894: FINERACT-1410 :Allow Payment types to map to all types of Ledgers

Posted by GitBox <gi...@apache.org>.

francisguchie commented on pull request #1894:
URL: https://github.com/apache/fineract/pull/1894#issuecomment-947213198


   Dear Victor,
   
   Most tests do not pass (at least for me) I think i have seen more than 10
   tests fail, so Ignore them most of the time. before you send a PR below are
   the steps you must do
   
   STEPS TAKEN
   1a- Add your Changes
   git add .
   
   1b- Apply Spotless
   ./gradlew spotlessApply
   
   2- Add your Changes
   git add .
   
   3- To Build War / Jar for deployment (any one below)
   ./gradlew clean bootWar  -- boots and builds war file
   ./gradlew clean war      -- builds war file
   ./gradlew bootJar        -- builds jar file
   
   Then you commit and push PR
   
   When these 3 sections pass on your PC then your PR should be OK
   
   
   
   My Kind Regards
   
   Francis Guchie Kirago
   *Skype:* francisguchie
   *Telegram: *232 79 19 44 07
   *Whatsapp: *232 79 19 44 07
   *LINKEDIN:* https://www.linkedin.com/in/francis-guchie-kirago-a4379617/
   twitter: @FrancisGuchie
   
   
   
   
   
   
   
   
   
   On Tue, Oct 19, 2021 at 7:49 PM Víctor Romero ***@***.***>
   wrote:
   
   > @ankita10r <https://github.com/ankita10r> @avikganguly01
   > <https://github.com/avikganguly01> in the testings I see these messages:
   >
   > org.apache.fineract.integrationtests.SkipRepaymentOnMonthFirstTest
   > Test testSkippingRepaymentOnFirstDayOfMonth() FAILED (3.8s)
   > org.opentest4j.AssertionFailedError: expected: <30> but was: <32>
   > at
   > app//org.apache.fineract.integrationtests.SkipRepaymentOnMonthFirstTest.tearDown(SkipRepaymentOnMonthFirstTest.java:71)
   > Test checkRepaymentSkipOnFirstDayOfMonth() SKIPPED
   > FAILURE: Executed 86 tests in 2m 16s (1 failed, 2 skipped)
   >
   > Are they expected?
   >
   > —
   > You are receiving this because you are subscribed to this thread.
   > Reply to this email directly, view it on GitHub
   > <https://github.com/apache/fineract/pull/1894#issuecomment-947053724>, or
   > unsubscribe
   > <https://github.com/notifications/unsubscribe-auth/AFNCABTVWJ7KN4EA477EXE3UHXDWJANCNFSM5GG2FFOQ>
   > .
   > Triage notifications on the go with GitHub Mobile for iOS
   > <https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675>
   > or Android
   > <https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>.
   >
   >
   


-- 
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.

To unsubscribe, e-mail: commits-unsubscribe@fineract.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [fineract] ptuomola commented on pull request #1894: FINERACT-1410 :Allow Payment types to map to all types of Ledgers

Posted by GitBox <gi...@apache.org>.

ptuomola commented on pull request #1894:
URL: https://github.com/apache/fineract/pull/1894#issuecomment-947239682


   @avikganguly01 @francisguchie @ankita10r @IOhacker 
   
   This has now broken the build, and all the other builds are failing. 
   We should _never_ merge failing builds please. 
   I will try to revert this change - please can you fix the failing tests before merging again. 
   


-- 
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.

To unsubscribe, e-mail: commits-unsubscribe@fineract.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [fineract] ptuomola edited a comment on pull request #1894: FINERACT-1410 :Allow Payment types to map to all types of Ledgers

Posted by GitBox <gi...@apache.org>.

ptuomola edited a comment on pull request #1894:
URL: https://github.com/apache/fineract/pull/1894#issuecomment-949113316


   And the reason for the current build failure is also clear in the logs: the Travis infra is under such a load that the new SonarQube check added by IOHacker is not completing in 10 minutes, and therefore the build is timing out. 
   
   So not a "random flaky test" but clear issue with a recently made change.  But I can also see that IOHacker has already put forward a fix for this in a separate PR. 


-- 
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.

To unsubscribe, e-mail: commits-unsubscribe@fineract.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [fineract] avikganguly01 commented on pull request #1894: FINERACT-1410 :Allow Payment types to map to all types of Ledgers

Posted by GitBox <gi...@apache.org>.

avikganguly01 commented on pull request #1894:
URL: https://github.com/apache/fineract/pull/1894#issuecomment-950615591


   @fintecheando (cc @ptuomola @awasum) : Thank you for adding SonarQube! Do you have any inputs for reducing test time 


-- 
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.

To unsubscribe, e-mail: commits-unsubscribe@fineract.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [fineract] ptuomola commented on pull request #1894: FINERACT-1410 :Allow Payment types to map to all types of Ledgers

Posted by GitBox <gi...@apache.org>.

ptuomola commented on pull request #1894:
URL: https://github.com/apache/fineract/pull/1894#issuecomment-949113316


   And the reason for the current build failure is also clear in the logs: the Travis infra is under such a load that our test scripts have not provided any output for 10 minutes, so Travis thinks the build has stalled. But I can also see that a fix for this has already been put forward in a separate PR. So yet another step towards making the tests reliable. 


-- 
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.

To unsubscribe, e-mail: commits-unsubscribe@fineract.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [fineract] ptuomola commented on pull request #1894: FINERACT-1410 :Allow Payment types to map to all types of Ledgers

Posted by GitBox <gi...@apache.org>.

ptuomola commented on pull request #1894:
URL: https://github.com/apache/fineract/pull/1894#issuecomment-947599748


   The failing test was exactly as pointed out by @IOhacker before this PR was merged:
   
   org.apache.fineract.integrationtests.SkipRepaymentOnMonthFirstTest
   Test testSkippingRepaymentOnFirstDayOfMonth() FAILED (3.8s)
   org.opentest4j.AssertionFailedError: expected: <30> but was: <32>
   at app//org.apache.fineract.integrationtests.SkipRepaymentOnMonthFirstTest.tearDown(SkipRepaymentOnMonthFirstTest.java:71)
   
   And as per our standards:
   
   'Acknowledge that we will not review PRs that are not passing the build ("green") - it is your responsibility to get a proposed PR to pass the build, not primarily the project's maintainers"


-- 
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.

To unsubscribe, e-mail: commits-unsubscribe@fineract.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [fineract] avikganguly01 merged pull request #1894: FINERACT-1410 :Allow Payment types to map to all types of Ledgers

Posted by GitBox <gi...@apache.org>.

avikganguly01 merged pull request #1894:
URL: https://github.com/apache/fineract/pull/1894


   


-- 
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.

To unsubscribe, e-mail: commits-unsubscribe@fineract.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [fineract] avikganguly01 commented on pull request #1894: FINERACT-1410 :Allow Payment types to map to all types of Ledgers

Posted by GitBox <gi...@apache.org>.

avikganguly01 commented on pull request #1894:
URL: https://github.com/apache/fineract/pull/1894#issuecomment-947587901


   > @avikganguly01 @francisguchie @ankita10r @IOhacker
   > 
   > This has now broken the build, and all the other builds are failing. We should _never_ merge failing builds please. I will try to revert this change - please can you fix the failing tests before merging again.
   
   @IOhacker : There are 10+ tests like testSkippingRepaymentOnFirstDayOfMonth() which fails randomly and has nothing to do with this PR.
   
   @ptuomola : Can you be more specific with regards to which integration test failed because of this PR? Do not revert this change unless proven otherwise. The author of this PR is not responsible for fixing all the integration tests which fail due to no fault of hers.
   
   


-- 
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.

To unsubscribe, e-mail: commits-unsubscribe@fineract.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [fineract] IOhacker commented on pull request #1894: FINERACT-1410 :Allow Payment types to map to all types of Ledgers

Posted by GitBox <gi...@apache.org>.

IOhacker commented on pull request #1894:
URL: https://github.com/apache/fineract/pull/1894#issuecomment-947053724


   @ankita10r @avikganguly01 in the testings I see these messages:
   
   org.apache.fineract.integrationtests.SkipRepaymentOnMonthFirstTest
     Test testSkippingRepaymentOnFirstDayOfMonth() FAILED (3.8s)
     org.opentest4j.AssertionFailedError: expected: <30> but was: <32>
         at app//org.apache.fineract.integrationtests.SkipRepaymentOnMonthFirstTest.tearDown(SkipRepaymentOnMonthFirstTest.java:71)
     Test checkRepaymentSkipOnFirstDayOfMonth() SKIPPED
   FAILURE: Executed 86 tests in 2m 16s (1 failed, 2 skipped)
   
   Are they expected?
   


-- 
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.

To unsubscribe, e-mail: commits-unsubscribe@fineract.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [fineract] avikganguly01 edited a comment on pull request #1894: FINERACT-1410 :Allow Payment types to map to all types of Ledgers

Posted by GitBox <gi...@apache.org>.

avikganguly01 edited a comment on pull request #1894:
URL: https://github.com/apache/fineract/pull/1894#issuecomment-948901095

> All the PRs were failing with the same error.

Thank you for spotting and highlighting this. This doesn't help authors or reviewers from preventing the problem.

> But the point is: it should be the responsibility of the person submitting the PR to ensure the build passes before it is submitted, not for us to prove that the failure is due to their PR.

Example of one such tedious flow :

The build is failing again without any clear reason : -
https://app.travis-ci.com/github/apache/fineract/builds/240299730

--
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.

To unsubscribe, e-mail: commits-unsubscribe@fineract.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [fineract] ptuomola commented on pull request #1894: FINERACT-1410 :Allow Payment types to map to all types of Ledgers

Posted by GitBox <gi...@apache.org>.

ptuomola commented on pull request #1894:
URL: https://github.com/apache/fineract/pull/1894#issuecomment-949110683

Hi @avikganguly01 (cc @awasum @fynmanoj)

Unfortunately I disagree with your description of the current state of the Fineract branch. Looking at the pull request history, I've merged around 350 PRs over the two years or so. For each of these, I've ensured all the tests pass before merging. So, in my view, this is not an unrealistic expectation. In fact, this is the only way we can make sure things improve rather than degrade.

I agree there are still some flaky tests. But we've made a huge amount of progress on this over the last year: I've fixed at least 30 such tests myself. There are still a couple of known issues with:

a) performance of Travis (sometimes the build takes too long due to load on the Travis infra, and some tests time out)

b) month end (some tests fail on last / first day of months, due to the fact that different months have different number of days and this confuses the checking routines for interest calculation)

I agree we should fix these. But these handful of issues should not be an excuse for allowing code to be merged that breaks additional tests, without even investigating why the tests are failing. That will take us even further away from our goal of having reliable 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.

To unsubscribe, e-mail: commits-unsubscribe@fineract.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [fineract] IOhacker commented on pull request #1894: FINERACT-1410 :Allow Payment types to map to all types of Ledgers

Posted by GitBox <gi...@apache.org>.

IOhacker commented on pull request #1894:
URL: https://github.com/apache/fineract/pull/1894#issuecomment-951422766


   I am reading how to improve the testing execution time, because the tests
   themselves are consuming most of the time during the Travis-Ci job task
   execution.
   
   As reference:
   
   https://docs.gradle.org/7.2/userguide/java_testing.html#sec:test_execution
   https://junit.org/junit5/docs/current/user-guide/#writing-tests-parallel-execution
   
   
   Regards
   
   Victor
   
   El lun, 25 oct 2021 a las 2:39, Avik Ganguly ***@***.***>)
   escribió:
   
   > @fintecheando <https://github.com/fintecheando> (cc @ptuomola
   > <https://github.com/ptuomola> @awasum <https://github.com/awasum>) :
   > Thank you for adding SonarQube! Do you have any inputs for reducing test
   > time
   >
   > —
   > You are receiving this because you were mentioned.
   > Reply to this email directly, view it on GitHub
   > <https://github.com/apache/fineract/pull/1894#issuecomment-950615591>, or
   > unsubscribe
   > <https://github.com/notifications/unsubscribe-auth/ALD2ZATPFOZSG3LMDEYMIBDUIUCTPANCNFSM5GG2FFOQ>
   > .
   > Triage notifications on the go with GitHub Mobile for iOS
   > <https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675>
   > or Android
   > <https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>.
   >
   >
   


-- 
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.

To unsubscribe, e-mail: commits-unsubscribe@fineract.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org