avikganguly01 edited a comment 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. What are your thoughts on this? > 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 : 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
