Lucene - Core
  1. Lucene - Core
  2. LUCENE-4160

Bring back the functional equivalent of tests.iters.min

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Trivial Trivial
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 4.0-BETA, 6.0
    • Component/s: general/test
    • Labels:
      None
    • Lucene Fields:
      New

      Description

      What is needed is effectively saying: "repeat this test N times, but stop once you hit a failure".

      Previously it was "tests.iters.min=X" which is (still) kind of confusing to me because I don't understand how "X" is related to the original question.

      I propose to implement a boolean "tests.fastfail" which would ignore any tests running on the same JVM after the first failure has been hit.

      Those with fond memories of "tests.iters.min" speak up, please.

        Activity

        Hide
        Michael McCandless added a comment -

        I propose to implement a boolean "tests.fastfail"

        +1, I think that's a nice simplification over tests.iters.min.

        Show
        Michael McCandless added a comment - I propose to implement a boolean "tests.fastfail" +1, I think that's a nice simplification over tests.iters.min.
        Hide
        Hoss Man added a comment -

        "tests.iters.min=X" which is (still) kind of confusing to me because I don't understand how "X" is related to the original question.

        i think maybe you simplified the statement of the original question?

        With tests.iters.min=X tests.iters=Y you were saying "attempt to run this test Y times, and even if it fails, run it a minimum of X times." (at least, that was the theory as i understood it, maybe it never actually worked that way).

        I propose to implement a boolean "tests.fastfail"

        ...the key here being that tests.iters today already repeats the test the specified number of times, even if it fails – and a new tests.fastfail" would default to "false" but if it was true then you could have the old behavior of tests.iters.min=1.

        seems fine to me ... i think hte main motivation of tests.iters.min isn't really applicable anymore since the default logic for running multiple iterations is basically the reverse of what it use to be.

        an alternative way to think about it would be to add a "tests.iters.max", since tests.iters already behaves similar to what tests.iters.min use to do.

        Or just say to hell with it, and people who want the X,Y equivilent from before can run...

        ant -Dtestcase=... -Dtestmethod=... -Dtests.iters=X \
        && \
        ant -Dtestcase=... -Dtestmethod=... -Dtests.failfast -Dtests.iters=${Y-X}
        
        Show
        Hoss Man added a comment - "tests.iters.min=X" which is (still) kind of confusing to me because I don't understand how "X" is related to the original question. i think maybe you simplified the statement of the original question? With tests.iters.min=X tests.iters=Y you were saying "attempt to run this test Y times, and even if it fails, run it a minimum of X times." (at least, that was the theory as i understood it, maybe it never actually worked that way). I propose to implement a boolean "tests.fastfail" ...the key here being that tests.iters today already repeats the test the specified number of times, even if it fails – and a new tests.fastfail" would default to "false" but if it was true then you could have the old behavior of tests.iters.min=1. seems fine to me ... i think hte main motivation of tests.iters.min isn't really applicable anymore since the default logic for running multiple iterations is basically the reverse of what it use to be. an alternative way to think about it would be to add a "tests.iters.max", since tests.iters already behaves similar to what tests.iters.min use to do. Or just say to hell with it, and people who want the X,Y equivilent from before can run... ant -Dtestcase=... -Dtestmethod=... -Dtests.iters=X \ && \ ant -Dtestcase=... -Dtestmethod=... -Dtests.failfast -Dtests.iters=${Y-X}
        Hide
        Dawid Weiss added a comment -

        Thanks for the clarification, Hoss. We could also do:

        -Dtests.iters=X -Dtests.minfailures=Y
        

        What this would do is repeat everything X times but ignore anything after the first Y failures... So if you'd like to quickly abort after the first failure, you'd do:

        -Dtests.iters=X -Dtests.minfailures=1
        

        and if you wanted more (for whatever reason) you could wait for more, but still abort earlier than X. I'll just provide a patch for this and we'll see how it turns out in practice.

        Show
        Dawid Weiss added a comment - Thanks for the clarification, Hoss. We could also do: -Dtests.iters=X -Dtests.minfailures=Y What this would do is repeat everything X times but ignore anything after the first Y failures... So if you'd like to quickly abort after the first failure, you'd do: -Dtests.iters=X -Dtests.minfailures=1 and if you wanted more (for whatever reason) you could wait for more, but still abort earlier than X. I'll just provide a patch for this and we'll see how it turns out in practice.
        Hide
        Dawid Weiss added a comment -

        Now that I think of it minfailures or maxfailures depends on how you look at the problem

        Show
        Dawid Weiss added a comment - Now that I think of it minfailures or maxfailures depends on how you look at the problem
        Hide
        Dawid Weiss added a comment -

        Patch against trunk.

        ant -Dtests.maxfailures=M
        

        will cause any test after the first M failures to be assumption-ignored. Example:

           [junit4] IGNOR/A 0.03s | TestBuhu.testFailSometimes {#96 seed=[15604D6381DA415B:BE5164F081683030]}
           [junit4]    > Assumption #1: Ignored, failures limit reached (1 >= 1).
           [junit4] IGNOR/A 0.01s | TestBuhu.testFailSometimes {#97 seed=[15604D6381DA415B:E515E49B8D52E84E]}
           [junit4]    > Assumption #1: Ignored, failures limit reached (1 >= 1).
           [junit4] IGNOR/A 0.02s | TestBuhu.testFailSometimes {#98 seed=[15604D6381DA415B:2128D6F71D04F74D]}
           [junit4]    > Assumption #1: Ignored, failures limit reached (1 >= 1).
           [junit4] IGNOR/A 0.01s | TestBuhu.testFailSometimes {#99 seed=[15604D6381DA415B:EBBC938E1A402A5E]}
           [junit4]    > Assumption #1: Ignored, failures limit reached (1 >= 1).
           [junit4]    > (@AfterClass output)
           [junit4]   2> NOTE: test params are: codec=Lucene40: {}, sim=RandomSimilarityProvider(queryNorm=false,coord=true): {}, locale=hu, timezone=US/Arizona
           [junit4]   2> NOTE: Linux 3.0.0-21-generic amd64/Oracle Corporation 1.7.0 (64-bit)/cpus=2,threads=1,free=50888552,total=78512128
           [junit4]   2> NOTE: All tests run in this JVM: [TestBuhu]
           [junit4]   2> 
           [junit4] Completed in 1.44s, 100 tests, 1 failure, 91 skipped <<< FAILURES!
           [junit4]  
           [junit4] JVM J0:     0.70 ..     3.16 =     2.47s
           [junit4] Execution time total: 3.21 sec.
           [junit4] Tests summary: 1 suite, 100 tests, 1 failure, 91 ignored (91 assumptions)
        
        Show
        Dawid Weiss added a comment - Patch against trunk. ant -Dtests.maxfailures=M will cause any test after the first M failures to be assumption-ignored. Example: [junit4] IGNOR/A 0.03s | TestBuhu.testFailSometimes {#96 seed=[15604D6381DA415B:BE5164F081683030]} [junit4] > Assumption #1: Ignored, failures limit reached (1 >= 1). [junit4] IGNOR/A 0.01s | TestBuhu.testFailSometimes {#97 seed=[15604D6381DA415B:E515E49B8D52E84E]} [junit4] > Assumption #1: Ignored, failures limit reached (1 >= 1). [junit4] IGNOR/A 0.02s | TestBuhu.testFailSometimes {#98 seed=[15604D6381DA415B:2128D6F71D04F74D]} [junit4] > Assumption #1: Ignored, failures limit reached (1 >= 1). [junit4] IGNOR/A 0.01s | TestBuhu.testFailSometimes {#99 seed=[15604D6381DA415B:EBBC938E1A402A5E]} [junit4] > Assumption #1: Ignored, failures limit reached (1 >= 1). [junit4] > (@AfterClass output) [junit4] 2> NOTE: test params are: codec=Lucene40: {}, sim=RandomSimilarityProvider(queryNorm=false,coord=true): {}, locale=hu, timezone=US/Arizona [junit4] 2> NOTE: Linux 3.0.0-21-generic amd64/Oracle Corporation 1.7.0 (64-bit)/cpus=2,threads=1,free=50888552,total=78512128 [junit4] 2> NOTE: All tests run in this JVM: [TestBuhu] [junit4] 2> [junit4] Completed in 1.44s, 100 tests, 1 failure, 91 skipped <<< FAILURES! [junit4] [junit4] JVM J0: 0.70 .. 3.16 = 2.47s [junit4] Execution time total: 3.21 sec. [junit4] Tests summary: 1 suite, 100 tests, 1 failure, 91 ignored (91 assumptions)
        Hide
        Dawid Weiss added a comment -

        One note – this property is per-jvm-global in the sense that you don't need -Dtests.iters, you can wait for M failures in general, so for example:

        ant test-core -Dtests.maxfailures=1
        

        will ignore any remaining tests after the first failure. This applies per-JVM though, so if you're running with > 1 fork ed JVM then only that JVM's tests will be ignored.

        Show
        Dawid Weiss added a comment - One note – this property is per-jvm-global in the sense that you don't need -Dtests.iters, you can wait for M failures in general, so for example: ant test-core -Dtests.maxfailures=1 will ignore any remaining tests after the first failure. This applies per-JVM though, so if you're running with > 1 fork ed JVM then only that JVM's tests will be ignored.
        Hide
        Robert Muir added a comment -

        What is the use case of the min/maxfailures (besides just your idea of a fastfail boolean, which I liked)

        Show
        Robert Muir added a comment - What is the use case of the min/maxfailures (besides just your idea of a fastfail boolean, which I liked)
        Hide
        Dawid Weiss added a comment -

        Maybe somebody would want to wait for 5 failures instead of 1 to get a bunch of stack traces? A boolean can be emulated at ant level, it's basically -Dtests.maxfailures=1... Don't know, really – I admit I don't need this so I'm shooting at the dark here, it seemed to be useful for folks. To me it can be a boolean as well.

        Show
        Dawid Weiss added a comment - Maybe somebody would want to wait for 5 failures instead of 1 to get a bunch of stack traces? A boolean can be emulated at ant level, it's basically -Dtests.maxfailures=1... Don't know, really – I admit I don't need this so I'm shooting at the dark here, it seemed to be useful for folks. To me it can be a boolean as well.
        Hide
        Robert Muir added a comment -

        I think i originally caused the complexity by wanting to still have a way to run a test like 1000 times and look at the failure rate. This is occasionally useful: e.g. this test fails 2% of the time and I improved the test to fail 10% of the time or whatever

        But really I think its more useful in general to have 'fastfail' on by default... especially now that in general tests are reproducing a lot better than before...

        Show
        Robert Muir added a comment - I think i originally caused the complexity by wanting to still have a way to run a test like 1000 times and look at the failure rate. This is occasionally useful: e.g. this test fails 2% of the time and I improved the test to fail 10% of the time or whatever But really I think its more useful in general to have 'fastfail' on by default... especially now that in general tests are reproducing a lot better than before...
        Hide
        Hoss Man added a comment -

        I think i originally caused the complexity by wanting to still have a way to run a test like 1000 times and look at the failure rate.

        right, the original driving usecase is already possible with the way tests.iters works now; it just so happened that the way it worked before you could not only say "run this test at least 1000 times, even if it fails, so i can compute a pass/fail rate and look for patterns" you could say "try to run this test at least 1000 times, even if it fails, so i can compute a pass/fail rate and look for patterns – but if it doesn't fail, just keep on trying up to 5000 times for good measure. I'm going to lunch anyway."

        if tests.maxfailures is just as easy to implement as tests.fastfail (and already implemented in this patch) then i say go with that.

        we can always add ant sugar so that -Dtests.failfast=true sets tests.maxfailures=1

        Show
        Hoss Man added a comment - I think i originally caused the complexity by wanting to still have a way to run a test like 1000 times and look at the failure rate. right, the original driving usecase is already possible with the way tests.iters works now; it just so happened that the way it worked before you could not only say "run this test at least 1000 times, even if it fails, so i can compute a pass/fail rate and look for patterns" you could say "try to run this test at least 1000 times, even if it fails, so i can compute a pass/fail rate and look for patterns – but if it doesn't fail, just keep on trying up to 5000 times for good measure. I'm going to lunch anyway." if tests.maxfailures is just as easy to implement as tests.fastfail (and already implemented in this patch) then i say go with that. we can always add ant sugar so that -Dtests.failfast=true sets tests.maxfailures=1
        Hide
        Dawid Weiss added a comment -

        Yes, this patch is already working. I'll make it an alias so that -Dtests.failfast=[on/true/yes] will also work and commit it in.

        Show
        Dawid Weiss added a comment - Yes, this patch is already working. I'll make it an alias so that -Dtests.failfast= [on/true/yes] will also work and commit it in.
        Hide
        Hoss Man added a comment -

        hoss20120711-manual-post-40alpha-change

        Show
        Hoss Man added a comment - hoss20120711-manual-post-40alpha-change

          People

          • Assignee:
            Dawid Weiss
            Reporter:
            Dawid Weiss
          • Votes:
            0 Vote for this issue
            Watchers:
            3 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development