You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@fineract.apache.org by Saransh Sharma <sa...@muellners.com> on 2020/05/02 16:16:32 UTC
Re: New Policy about how to deal with unstable tests
I investigated these tests, they are failing because of the interdependent
nature of the application, Ignoring them eventually is not a better idea.
Since the nature of the "flaky" test is unknown , are we sure that we might
not encounter the same problem? in the next PR or in the future PR's
Points :
1. I made some changes not related to anything to other packages
Failed 3 of the tests . :) Pretty crazy , I think the status from the API
is also returning 403 (is this normal don't think so.)
https://api.travis-ci.org/v3/job/682216325/log.txt (Is this normal
behaviour , plus local its quite different)
Thus leading to this , "The following objects may have been concurrently
modified in another transaction" It is clear that at the same time the
transactions are happening.
I would suggest that we group tests based on the packages and then perform
functional tests based on the order of the dependency.
Let me know, if at all we should try to resolve those test issues rather
than putting Ignore.
On Tue, Apr 28, 2020 at 10:00 AM James Dailey <ja...@gmail.com>
wrote:
> +1
>
> "Please do NOT just @Ignore any existing tests mixed in as part of your
> larger PR."
>
> This seems like a key point to bring up to a higher level in the code
> review. i.e. should there be a search for @Ignore when doing a code
> review?
>
>
>
>
>
>
>
>
> On Sun, Apr 26, 2020 at 2:30 AM Michael Vorburger <mi...@vorburger.ch>
> wrote:
>
>> Hello everyone,
>>
>> I propose https://github.com/apache/fineract/pull/782 as our New Policy
>> about how to deal with unstable tests (text proposed to be added to the
>> README in that PR is copy/pasted below for your reading convenience).
>>
>> Tx,
>> M.
>>
>> If your PR is failing to pass our CI build due to a test failure which
>> you suspect is a "flaky" test, and not due to a change in your PR, then
>> please do not simply wait for an active maintainer to come and help you,
>> but instead be a proactive contributor to the project, like this: Search
>> for the name of the failed test on https://issues.apache.org/jira/, e.g.
>> for AccountingScenarioIntegrationTest you would find FINERACT-899
>> <https://issues.apache.org/jira/browse/FINERACT-899>. If you find
>> previous comments "proving" that the same test has arbitrarily failed in at
>> least 3 past PRs, then please do yourself raise a small separate new PR
>> proposing to add an @Ignore to the respective unstable test (e.g. #774
>> <https://github.com/apache/fineract/pull/774>) with the commit message
>> mentioning said JIRA (as always). Please do NOT just @Ignore any
>> existing tests mixed in as part of your larger PR. If there is no existing
>> JIRA for the test, then first please evaluate whether the failure couldn't
>> be a (perhaps strange) impact of the change you are proposing after all. If
>> it's not, then please raise a new JIRA to document the suspected Flaky
>> Test, and link it to FINERACT-850
>> <https://issues.apache.org/jira/browse/FINERACT-850>. This will allow
>> the next person coming along hitting the same test failure to easily find
>> it, and eventually propose to ignore the unstable test. Then (only) Close
>> and Reopen your PR, which will cause a new build, to see if it passes.
>>
>> _______________________
>> Michael Vorburger
>> http://www.vorburger.ch
>>
>
--
Saransh Sharma
*Research Partner*
*Muellner Internet Pvt Ltd *
----------------------------------------------------------------------------------------------
The idea of separation of me and you is amazing.
----------------------------------------------------------------------------------------------
*Company Website <https://www.muellners.com/> **Company Linkedin
<https://www.linkedin.com/company/muellners> *Company Facebook
<https://www.facebook.com/muellners>
This mail is governed by Muellner® Internet Private Limited's IT policy.
The information contained in this e-mail and any accompanying documents may
contain information that is confidential or otherwise protected from
disclosure. If you are not the intended recipient of this message, or if
this message has been addressed to you in error, please immediately alert
the sender by reply e-mail and then delete this message, including any
attachments. Any dissemination, distribution or other use of the contents
of this message by anyone other than the intended recipient is strictly
prohibited. All messages sent to and from this e-mail address may be
monitored as permitted by applicable law and regulations to ensure
compliance with our internal policies and to protect our business. E-mails
are not secure and cannot be guaranteed to be error free as they can be
intercepted, amended, lost or destroyed, or contain viruses. You are deemed
to have accepted these risks if you communicate with us by e-mail.
Re: New Policy about how to deal with unstable tests
Posted by Michael Vorburger <mi...@vorburger.ch>.
So I've had some "fun" on this front, and propose
https://github.com/apache/fineract/pull/817, which is the first step to
completely change how our integration tests tests await completion of
background jobs... code review feedback welcome!
On Sun, May 3, 2020 at 1:57 PM Awasum Yannick <aw...@apache.org> wrote:
> Full disclosure, Petri first noticed this here
> <https://issues.apache.org/jira/browse/FINERACT-855?focusedCommentId=17098277&page=com.atlassian.jira.plugin.system.issuetabpanels%3Acomment-tabpanel#comment-17098277>
>
> Also see: https://issues.apache.org/jira/browse/FINERACT-922
>
> On Sun, May 3, 2020 at 12:18 PM Awasum Yannick <aw...@apache.org> wrote:
>
>> Looks like the flaky Integration tests failures is as a result of
>> schedule jobs taking too much time to complete so much that by the time an
>> assertion is done, the job history has zero completed jobs and we get in
>> some cases array out of bound exceptions due to 0 - 1 index. And in some
>> cases assertion errors.
>>
>> I will soon be sending a pr based on this and comments made else where.
>>
>> PS: sorry, am on mobile
>>
>> On Sat, May 2, 2020, 18:00 Michael Vorburger <mi...@vorburger.ch> wrote:
>>
>>> Saransh,
>>>
>>> yes, totally agree, of course fixing any failing test is the best, and
>>> any contributions for that would be very helpful.
>>>
>>> The point we are making with
>>> https://github.com/apache/fineract#pull-requests is obviously not ;)
>>> that we don't welcome help with fixing flaky tests, but simply that we
>>> cannot block people's PRs on anything else due to clearly unstable ITs. A
>>> project cannot stop parallel work and just grind to a complete halt because
>>> of some failing tests - as important as fixing failing tests ASAP is, yes,
>>> of course.
>>>
>>> I do meanwhile firmly believe that ignoring tests, IFF they are proven
>>> to be unrelated to a change proposed in a PR (as the README states), and
>>> then having separate investigations by interested parties to re-un-ignore
>>> any fixed tests, is the right strategy. Hope you agree?
>>>
>>> Best,
>>> M.
>>> _______________________
>>> Michael Vorburger
>>> http://www.vorburger.ch
>>>
>>>
>>> On Sat, May 2, 2020 at 6:17 PM Saransh Sharma <sa...@muellners.com>
>>> wrote:
>>>
>>>>
>>>> I investigated these tests, they are failing because of the
>>>> interdependent nature of the application, Ignoring them eventually is not a
>>>> better idea. Since the nature of the "flaky" test is unknown , are we sure
>>>> that we might not encounter the same problem? in the next PR or in the
>>>> future PR's
>>>>
>>>> Points :
>>>>
>>>> 1. I made some changes not related to anything to other packages
>>>>
>>>> Failed 3 of the tests . :) Pretty crazy , I think the status from the
>>>> API is also returning 403 (is this normal don't think so.)
>>>>
>>>> https://api.travis-ci.org/v3/job/682216325/log.txt (Is this normal
>>>> behaviour , plus local its quite different)
>>>>
>>>> Thus leading to this , "The following objects may have been
>>>> concurrently modified in another transaction" It is clear that at the same
>>>> time the transactions are happening.
>>>> I would suggest that we group tests based on the packages and then
>>>> perform functional tests based on the order of the dependency.
>>>>
>>>> Let me know, if at all we should try to resolve those test issues
>>>> rather than putting Ignore.
>>>>
>>>> On Tue, Apr 28, 2020 at 10:00 AM James Dailey <ja...@gmail.com>
>>>> wrote:
>>>>
>>>>> +1
>>>>>
>>>>> "Please do NOT just @Ignore any existing tests mixed in as part of
>>>>> your larger PR."
>>>>>
>>>>> This seems like a key point to bring up to a higher level in the code
>>>>> review. i.e. should there be a search for @Ignore when doing a code
>>>>> review?
>>>>>
>>>>>
>>>>>
>>>>>
>>>>>
>>>>>
>>>>>
>>>>>
>>>>> On Sun, Apr 26, 2020 at 2:30 AM Michael Vorburger <mi...@vorburger.ch>
>>>>> wrote:
>>>>>
>>>>>> Hello everyone,
>>>>>>
>>>>>> I propose https://github.com/apache/fineract/pull/782 as our New
>>>>>> Policy about how to deal with unstable tests (text proposed to be added to
>>>>>> the README in that PR is copy/pasted below for your reading convenience).
>>>>>>
>>>>>> Tx,
>>>>>> M.
>>>>>>
>>>>>> If your PR is failing to pass our CI build due to a test failure
>>>>>> which you suspect is a "flaky" test, and not due to a change in your PR,
>>>>>> then please do not simply wait for an active maintainer to come and help
>>>>>> you, but instead be a proactive contributor to the project, like this:
>>>>>> Search for the name of the failed test on
>>>>>> https://issues.apache.org/jira/, e.g. for
>>>>>> AccountingScenarioIntegrationTest you would find FINERACT-899
>>>>>> <https://issues.apache.org/jira/browse/FINERACT-899>. If you find
>>>>>> previous comments "proving" that the same test has arbitrarily failed in at
>>>>>> least 3 past PRs, then please do yourself raise a small separate new PR
>>>>>> proposing to add an @Ignore to the respective unstable test (e.g.
>>>>>> #774 <https://github.com/apache/fineract/pull/774>) with the commit
>>>>>> message mentioning said JIRA (as always). Please do NOT just @Ignore
>>>>>> any existing tests mixed in as part of your larger PR. If there is no
>>>>>> existing JIRA for the test, then first please evaluate whether the failure
>>>>>> couldn't be a (perhaps strange) impact of the change you are proposing
>>>>>> after all. If it's not, then please raise a new JIRA to document the
>>>>>> suspected Flaky Test, and link it to FINERACT-850
>>>>>> <https://issues.apache.org/jira/browse/FINERACT-850>. This will
>>>>>> allow the next person coming along hitting the same test failure to easily
>>>>>> find it, and eventually propose to ignore the unstable test. Then (only)
>>>>>> Close and Reopen your PR, which will cause a new build, to see if it passes.
>>>>>>
>>>>>> _______________________
>>>>>> Michael Vorburger
>>>>>> http://www.vorburger.ch
>>>>>>
>>>>>
>>>>
>>>> --
>>>>
>>>> Saransh Sharma
>>>> *Research Partner*
>>>> *Muellner Internet Pvt Ltd *
>>>>
>>>>
>>>> ----------------------------------------------------------------------------------------------
>>>>
>>>> The idea of separation of me and you is amazing.
>>>>
>>>> ----------------------------------------------------------------------------------------------
>>>> *Company Website <https://www.muellners.com/> **Company Linkedin
>>>> <https://www.linkedin.com/company/muellners> *
>>>> Company Facebook <https://www.facebook.com/muellners>
>>>>
>>>> This mail is governed by Muellner® Internet Private Limited's IT
>>>> policy.
>>>> The information contained in this e-mail and any accompanying documents
>>>> may contain information that is confidential or otherwise protected from
>>>> disclosure. If you are not the intended recipient of this message, or if
>>>> this message has been addressed to you in error, please immediately alert
>>>> the sender by reply e-mail and then delete this message, including any
>>>> attachments. Any dissemination, distribution or other use of the contents
>>>> of this message by anyone other than the intended recipient is strictly
>>>> prohibited. All messages sent to and from this e-mail address may be
>>>> monitored as permitted by applicable law and regulations to ensure
>>>> compliance with our internal policies and to protect our business. E-mails
>>>> are not secure and cannot be guaranteed to be error free as they can be
>>>> intercepted, amended, lost or destroyed, or contain viruses. You are deemed
>>>> to have accepted these risks if you communicate with us by e-mail.
>>>>
>>>
Re: New Policy about how to deal with unstable tests
Posted by Awasum Yannick <aw...@apache.org>.
Full disclosure, Petri first noticed this here
<https://issues.apache.org/jira/browse/FINERACT-855?focusedCommentId=17098277&page=com.atlassian.jira.plugin.system.issuetabpanels%3Acomment-tabpanel#comment-17098277>
Also see: https://issues.apache.org/jira/browse/FINERACT-922
On Sun, May 3, 2020 at 12:18 PM Awasum Yannick <aw...@apache.org> wrote:
> Looks like the flaky Integration tests failures is as a result of schedule
> jobs taking too much time to complete so much that by the time an assertion
> is done, the job history has zero completed jobs and we get in some cases
> array out of bound exceptions due to 0 - 1 index. And in some cases
> assertion errors.
>
> I will soon be sending a pr based on this and comments made else where.
>
> PS: sorry, am on mobile
>
> On Sat, May 2, 2020, 18:00 Michael Vorburger <mi...@vorburger.ch> wrote:
>
>> Saransh,
>>
>> yes, totally agree, of course fixing any failing test is the best, and
>> any contributions for that would be very helpful.
>>
>> The point we are making with
>> https://github.com/apache/fineract#pull-requests is obviously not ;)
>> that we don't welcome help with fixing flaky tests, but simply that we
>> cannot block people's PRs on anything else due to clearly unstable ITs. A
>> project cannot stop parallel work and just grind to a complete halt because
>> of some failing tests - as important as fixing failing tests ASAP is, yes,
>> of course.
>>
>> I do meanwhile firmly believe that ignoring tests, IFF they are proven to
>> be unrelated to a change proposed in a PR (as the README states), and then
>> having separate investigations by interested parties to re-un-ignore any
>> fixed tests, is the right strategy. Hope you agree?
>>
>> Best,
>> M.
>> _______________________
>> Michael Vorburger
>> http://www.vorburger.ch
>>
>>
>> On Sat, May 2, 2020 at 6:17 PM Saransh Sharma <sa...@muellners.com>
>> wrote:
>>
>>>
>>> I investigated these tests, they are failing because of the
>>> interdependent nature of the application, Ignoring them eventually is not a
>>> better idea. Since the nature of the "flaky" test is unknown , are we sure
>>> that we might not encounter the same problem? in the next PR or in the
>>> future PR's
>>>
>>> Points :
>>>
>>> 1. I made some changes not related to anything to other packages
>>>
>>> Failed 3 of the tests . :) Pretty crazy , I think the status from the
>>> API is also returning 403 (is this normal don't think so.)
>>>
>>> https://api.travis-ci.org/v3/job/682216325/log.txt (Is this normal
>>> behaviour , plus local its quite different)
>>>
>>> Thus leading to this , "The following objects may have been
>>> concurrently modified in another transaction" It is clear that at the same
>>> time the transactions are happening.
>>> I would suggest that we group tests based on the packages and then
>>> perform functional tests based on the order of the dependency.
>>>
>>> Let me know, if at all we should try to resolve those test issues rather
>>> than putting Ignore.
>>>
>>> On Tue, Apr 28, 2020 at 10:00 AM James Dailey <ja...@gmail.com>
>>> wrote:
>>>
>>>> +1
>>>>
>>>> "Please do NOT just @Ignore any existing tests mixed in as part of
>>>> your larger PR."
>>>>
>>>> This seems like a key point to bring up to a higher level in the code
>>>> review. i.e. should there be a search for @Ignore when doing a code
>>>> review?
>>>>
>>>>
>>>>
>>>>
>>>>
>>>>
>>>>
>>>>
>>>> On Sun, Apr 26, 2020 at 2:30 AM Michael Vorburger <mi...@vorburger.ch>
>>>> wrote:
>>>>
>>>>> Hello everyone,
>>>>>
>>>>> I propose https://github.com/apache/fineract/pull/782 as our New
>>>>> Policy about how to deal with unstable tests (text proposed to be added to
>>>>> the README in that PR is copy/pasted below for your reading convenience).
>>>>>
>>>>> Tx,
>>>>> M.
>>>>>
>>>>> If your PR is failing to pass our CI build due to a test failure which
>>>>> you suspect is a "flaky" test, and not due to a change in your PR, then
>>>>> please do not simply wait for an active maintainer to come and help you,
>>>>> but instead be a proactive contributor to the project, like this: Search
>>>>> for the name of the failed test on https://issues.apache.org/jira/,
>>>>> e.g. for AccountingScenarioIntegrationTest you would find FINERACT-899
>>>>> <https://issues.apache.org/jira/browse/FINERACT-899>. If you find
>>>>> previous comments "proving" that the same test has arbitrarily failed in at
>>>>> least 3 past PRs, then please do yourself raise a small separate new PR
>>>>> proposing to add an @Ignore to the respective unstable test (e.g. #774
>>>>> <https://github.com/apache/fineract/pull/774>) with the commit
>>>>> message mentioning said JIRA (as always). Please do NOT just @Ignore
>>>>> any existing tests mixed in as part of your larger PR. If there is no
>>>>> existing JIRA for the test, then first please evaluate whether the failure
>>>>> couldn't be a (perhaps strange) impact of the change you are proposing
>>>>> after all. If it's not, then please raise a new JIRA to document the
>>>>> suspected Flaky Test, and link it to FINERACT-850
>>>>> <https://issues.apache.org/jira/browse/FINERACT-850>. This will allow
>>>>> the next person coming along hitting the same test failure to easily find
>>>>> it, and eventually propose to ignore the unstable test. Then (only) Close
>>>>> and Reopen your PR, which will cause a new build, to see if it passes.
>>>>>
>>>>> _______________________
>>>>> Michael Vorburger
>>>>> http://www.vorburger.ch
>>>>>
>>>>
>>>
>>> --
>>>
>>> Saransh Sharma
>>> *Research Partner*
>>> *Muellner Internet Pvt Ltd *
>>>
>>>
>>> ----------------------------------------------------------------------------------------------
>>>
>>> The idea of separation of me and you is amazing.
>>>
>>> ----------------------------------------------------------------------------------------------
>>> *Company Website <https://www.muellners.com/> **Company Linkedin
>>> <https://www.linkedin.com/company/muellners> *
>>> Company Facebook <https://www.facebook.com/muellners>
>>>
>>> This mail is governed by Muellner® Internet Private Limited's IT
>>> policy.
>>> The information contained in this e-mail and any accompanying documents
>>> may contain information that is confidential or otherwise protected from
>>> disclosure. If you are not the intended recipient of this message, or if
>>> this message has been addressed to you in error, please immediately alert
>>> the sender by reply e-mail and then delete this message, including any
>>> attachments. Any dissemination, distribution or other use of the contents
>>> of this message by anyone other than the intended recipient is strictly
>>> prohibited. All messages sent to and from this e-mail address may be
>>> monitored as permitted by applicable law and regulations to ensure
>>> compliance with our internal policies and to protect our business. E-mails
>>> are not secure and cannot be guaranteed to be error free as they can be
>>> intercepted, amended, lost or destroyed, or contain viruses. You are deemed
>>> to have accepted these risks if you communicate with us by e-mail.
>>>
>>
Re: New Policy about how to deal with unstable tests
Posted by Awasum Yannick <aw...@apache.org>.
Looks like the flaky Integration tests failures is as a result of schedule
jobs taking too much time to complete so much that by the time an assertion
is done, the job history has zero completed jobs and we get in some cases
array out of bound exceptions due to 0 - 1 index. And in some cases
assertion errors.
I will soon be sending a pr based on this and comments made else where.
PS: sorry, am on mobile
On Sat, May 2, 2020, 18:00 Michael Vorburger <mi...@vorburger.ch> wrote:
> Saransh,
>
> yes, totally agree, of course fixing any failing test is the best, and any
> contributions for that would be very helpful.
>
> The point we are making with
> https://github.com/apache/fineract#pull-requests is obviously not ;) that
> we don't welcome help with fixing flaky tests, but simply that we cannot
> block people's PRs on anything else due to clearly unstable ITs. A project
> cannot stop parallel work and just grind to a complete halt because of some
> failing tests - as important as fixing failing tests ASAP is, yes, of
> course.
>
> I do meanwhile firmly believe that ignoring tests, IFF they are proven to
> be unrelated to a change proposed in a PR (as the README states), and then
> having separate investigations by interested parties to re-un-ignore any
> fixed tests, is the right strategy. Hope you agree?
>
> Best,
> M.
> _______________________
> Michael Vorburger
> http://www.vorburger.ch
>
>
> On Sat, May 2, 2020 at 6:17 PM Saransh Sharma <sa...@muellners.com>
> wrote:
>
>>
>> I investigated these tests, they are failing because of the
>> interdependent nature of the application, Ignoring them eventually is not a
>> better idea. Since the nature of the "flaky" test is unknown , are we sure
>> that we might not encounter the same problem? in the next PR or in the
>> future PR's
>>
>> Points :
>>
>> 1. I made some changes not related to anything to other packages
>>
>> Failed 3 of the tests . :) Pretty crazy , I think the status from the API
>> is also returning 403 (is this normal don't think so.)
>>
>> https://api.travis-ci.org/v3/job/682216325/log.txt (Is this normal
>> behaviour , plus local its quite different)
>>
>> Thus leading to this , "The following objects may have been concurrently
>> modified in another transaction" It is clear that at the same time the
>> transactions are happening.
>> I would suggest that we group tests based on the packages and then
>> perform functional tests based on the order of the dependency.
>>
>> Let me know, if at all we should try to resolve those test issues rather
>> than putting Ignore.
>>
>> On Tue, Apr 28, 2020 at 10:00 AM James Dailey <ja...@gmail.com>
>> wrote:
>>
>>> +1
>>>
>>> "Please do NOT just @Ignore any existing tests mixed in as part of your
>>> larger PR."
>>>
>>> This seems like a key point to bring up to a higher level in the code
>>> review. i.e. should there be a search for @Ignore when doing a code
>>> review?
>>>
>>>
>>>
>>>
>>>
>>>
>>>
>>>
>>> On Sun, Apr 26, 2020 at 2:30 AM Michael Vorburger <mi...@vorburger.ch>
>>> wrote:
>>>
>>>> Hello everyone,
>>>>
>>>> I propose https://github.com/apache/fineract/pull/782 as our New
>>>> Policy about how to deal with unstable tests (text proposed to be added to
>>>> the README in that PR is copy/pasted below for your reading convenience).
>>>>
>>>> Tx,
>>>> M.
>>>>
>>>> If your PR is failing to pass our CI build due to a test failure which
>>>> you suspect is a "flaky" test, and not due to a change in your PR, then
>>>> please do not simply wait for an active maintainer to come and help you,
>>>> but instead be a proactive contributor to the project, like this: Search
>>>> for the name of the failed test on https://issues.apache.org/jira/,
>>>> e.g. for AccountingScenarioIntegrationTest you would find FINERACT-899
>>>> <https://issues.apache.org/jira/browse/FINERACT-899>. If you find
>>>> previous comments "proving" that the same test has arbitrarily failed in at
>>>> least 3 past PRs, then please do yourself raise a small separate new PR
>>>> proposing to add an @Ignore to the respective unstable test (e.g. #774
>>>> <https://github.com/apache/fineract/pull/774>) with the commit message
>>>> mentioning said JIRA (as always). Please do NOT just @Ignore any
>>>> existing tests mixed in as part of your larger PR. If there is no existing
>>>> JIRA for the test, then first please evaluate whether the failure couldn't
>>>> be a (perhaps strange) impact of the change you are proposing after all. If
>>>> it's not, then please raise a new JIRA to document the suspected Flaky
>>>> Test, and link it to FINERACT-850
>>>> <https://issues.apache.org/jira/browse/FINERACT-850>. This will allow
>>>> the next person coming along hitting the same test failure to easily find
>>>> it, and eventually propose to ignore the unstable test. Then (only) Close
>>>> and Reopen your PR, which will cause a new build, to see if it passes.
>>>>
>>>> _______________________
>>>> Michael Vorburger
>>>> http://www.vorburger.ch
>>>>
>>>
>>
>> --
>>
>> Saransh Sharma
>> *Research Partner*
>> *Muellner Internet Pvt Ltd *
>>
>>
>> ----------------------------------------------------------------------------------------------
>>
>> The idea of separation of me and you is amazing.
>>
>> ----------------------------------------------------------------------------------------------
>> *Company Website <https://www.muellners.com/> **Company Linkedin
>> <https://www.linkedin.com/company/muellners> *Company Facebook
>> <https://www.facebook.com/muellners>
>>
>> This mail is governed by Muellner® Internet Private Limited's IT policy.
>> The information contained in this e-mail and any accompanying documents
>> may contain information that is confidential or otherwise protected from
>> disclosure. If you are not the intended recipient of this message, or if
>> this message has been addressed to you in error, please immediately alert
>> the sender by reply e-mail and then delete this message, including any
>> attachments. Any dissemination, distribution or other use of the contents
>> of this message by anyone other than the intended recipient is strictly
>> prohibited. All messages sent to and from this e-mail address may be
>> monitored as permitted by applicable law and regulations to ensure
>> compliance with our internal policies and to protect our business. E-mails
>> are not secure and cannot be guaranteed to be error free as they can be
>> intercepted, amended, lost or destroyed, or contain viruses. You are deemed
>> to have accepted these risks if you communicate with us by e-mail.
>>
>
Re: New Policy about how to deal with unstable tests
Posted by Michael Vorburger <mi...@vorburger.ch>.
Saransh,
yes, totally agree, of course fixing any failing test is the best, and any
contributions for that would be very helpful.
The point we are making with
https://github.com/apache/fineract#pull-requests is obviously not ;) that
we don't welcome help with fixing flaky tests, but simply that we cannot
block people's PRs on anything else due to clearly unstable ITs. A project
cannot stop parallel work and just grind to a complete halt because of some
failing tests - as important as fixing failing tests ASAP is, yes, of
course.
I do meanwhile firmly believe that ignoring tests, IFF they are proven to
be unrelated to a change proposed in a PR (as the README states), and then
having separate investigations by interested parties to re-un-ignore any
fixed tests, is the right strategy. Hope you agree?
Best,
M.
_______________________
Michael Vorburger
http://www.vorburger.ch
On Sat, May 2, 2020 at 6:17 PM Saransh Sharma <sa...@muellners.com> wrote:
>
> I investigated these tests, they are failing because of the interdependent
> nature of the application, Ignoring them eventually is not a better idea.
> Since the nature of the "flaky" test is unknown , are we sure that we might
> not encounter the same problem? in the next PR or in the future PR's
>
> Points :
>
> 1. I made some changes not related to anything to other packages
>
> Failed 3 of the tests . :) Pretty crazy , I think the status from the API
> is also returning 403 (is this normal don't think so.)
>
> https://api.travis-ci.org/v3/job/682216325/log.txt (Is this normal
> behaviour , plus local its quite different)
>
> Thus leading to this , "The following objects may have been concurrently
> modified in another transaction" It is clear that at the same time the
> transactions are happening.
> I would suggest that we group tests based on the packages and then perform
> functional tests based on the order of the dependency.
>
> Let me know, if at all we should try to resolve those test issues rather
> than putting Ignore.
>
> On Tue, Apr 28, 2020 at 10:00 AM James Dailey <ja...@gmail.com>
> wrote:
>
>> +1
>>
>> "Please do NOT just @Ignore any existing tests mixed in as part of your
>> larger PR."
>>
>> This seems like a key point to bring up to a higher level in the code
>> review. i.e. should there be a search for @Ignore when doing a code
>> review?
>>
>>
>>
>>
>>
>>
>>
>>
>> On Sun, Apr 26, 2020 at 2:30 AM Michael Vorburger <mi...@vorburger.ch>
>> wrote:
>>
>>> Hello everyone,
>>>
>>> I propose https://github.com/apache/fineract/pull/782 as our New Policy
>>> about how to deal with unstable tests (text proposed to be added to the
>>> README in that PR is copy/pasted below for your reading convenience).
>>>
>>> Tx,
>>> M.
>>>
>>> If your PR is failing to pass our CI build due to a test failure which
>>> you suspect is a "flaky" test, and not due to a change in your PR, then
>>> please do not simply wait for an active maintainer to come and help you,
>>> but instead be a proactive contributor to the project, like this: Search
>>> for the name of the failed test on https://issues.apache.org/jira/,
>>> e.g. for AccountingScenarioIntegrationTest you would find FINERACT-899
>>> <https://issues.apache.org/jira/browse/FINERACT-899>. If you find
>>> previous comments "proving" that the same test has arbitrarily failed in at
>>> least 3 past PRs, then please do yourself raise a small separate new PR
>>> proposing to add an @Ignore to the respective unstable test (e.g. #774
>>> <https://github.com/apache/fineract/pull/774>) with the commit message
>>> mentioning said JIRA (as always). Please do NOT just @Ignore any
>>> existing tests mixed in as part of your larger PR. If there is no existing
>>> JIRA for the test, then first please evaluate whether the failure couldn't
>>> be a (perhaps strange) impact of the change you are proposing after all. If
>>> it's not, then please raise a new JIRA to document the suspected Flaky
>>> Test, and link it to FINERACT-850
>>> <https://issues.apache.org/jira/browse/FINERACT-850>. This will allow
>>> the next person coming along hitting the same test failure to easily find
>>> it, and eventually propose to ignore the unstable test. Then (only) Close
>>> and Reopen your PR, which will cause a new build, to see if it passes.
>>>
>>> _______________________
>>> Michael Vorburger
>>> http://www.vorburger.ch
>>>
>>
>
> --
>
> Saransh Sharma
> *Research Partner*
> *Muellner Internet Pvt Ltd *
>
>
> ----------------------------------------------------------------------------------------------
>
> The idea of separation of me and you is amazing.
>
> ----------------------------------------------------------------------------------------------
> *Company Website <https://www.muellners.com/> **Company Linkedin
> <https://www.linkedin.com/company/muellners> *Company Facebook
> <https://www.facebook.com/muellners>
>
> This mail is governed by Muellner® Internet Private Limited's IT policy.
> The information contained in this e-mail and any accompanying documents
> may contain information that is confidential or otherwise protected from
> disclosure. If you are not the intended recipient of this message, or if
> this message has been addressed to you in error, please immediately alert
> the sender by reply e-mail and then delete this message, including any
> attachments. Any dissemination, distribution or other use of the contents
> of this message by anyone other than the intended recipient is strictly
> prohibited. All messages sent to and from this e-mail address may be
> monitored as permitted by applicable law and regulations to ensure
> compliance with our internal policies and to protect our business. E-mails
> are not secure and cannot be guaranteed to be error free as they can be
> intercepted, amended, lost or destroyed, or contain viruses. You are deemed
> to have accepted these risks if you communicate with us by e-mail.
>