Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 4.0-ALPHA
    • Component/s: None
    • Labels:
      None

      Description

      Currently, Test2BTerms has an @Ignore annotation which means that the only way to run it as a test is to edit the file.

      There are a couple of options to fix this:

      1. Add a main() so it can be invoked via the command line outside of the test framework
      2. Add some new annotations that mark it as slow or weekly or something like that and have the test target ignore @slow (or whatever) by default, but can also turn it on.
      1. LUCENE-3615.patch
        10 kB
        Grant Ingersoll
      2. LUCENE-3615.patch
        10 kB
        Grant Ingersoll
      3. LUCENE-3615.patch
        11 kB
        Grant Ingersoll

        Activity

        Hide
        Hoss Man added a comment -

        Because of this, all of the tests behave in totally different ways that you cant really assign a weight to, e.g. take a look at the history of test times for this test:

        "Weight" may be the wrong term... i wasn't suggesting that it would be any sort of quantitative, comparable metric of how long the test would take – my point was just that having a numeric annotation where bigger means "this test does more stuff" would allow people to run more or less tests as they see fit with simple configuration, regardless of whether their idea of a test to be run "Nightly" jives directly with the @Nightly annotation (maybe i want to only run @Nightly tests on weekends?)

        As things stand, we have regular tests, and then we have @Nightly tests, and then we have @Slow tests ... hypothetically: if we add a new test later that's not nearly as bad as Test2BTerms, so we still want it to run as part of a "full test run" but is bad enough that we don't want to jenkins to do it was part of our "@Nightly" run, we have to consider some intermediate "@SortOfSlow" attribute ... hence my suggestion that instead of adding more special case annotations (and more build params for deciding when to exececute what), we just use an arbitrary range of numbers and two simple "min" and "max" build params to pick the tests to run.

        ...anyway ... it was just an idea.

        Show
        Hoss Man added a comment - Because of this, all of the tests behave in totally different ways that you cant really assign a weight to, e.g. take a look at the history of test times for this test: "Weight" may be the wrong term... i wasn't suggesting that it would be any sort of quantitative, comparable metric of how long the test would take – my point was just that having a numeric annotation where bigger means "this test does more stuff" would allow people to run more or less tests as they see fit with simple configuration, regardless of whether their idea of a test to be run "Nightly" jives directly with the @Nightly annotation (maybe i want to only run @Nightly tests on weekends?) As things stand, we have regular tests, and then we have @Nightly tests, and then we have @Slow tests ... hypothetically: if we add a new test later that's not nearly as bad as Test2BTerms, so we still want it to run as part of a "full test run" but is bad enough that we don't want to jenkins to do it was part of our "@Nightly" run, we have to consider some intermediate "@SortOfSlow" attribute ... hence my suggestion that instead of adding more special case annotations (and more build params for deciding when to exececute what), we just use an arbitrary range of numbers and two simple "min" and "max" build params to pick the tests to run. ...anyway ... it was just an idea.
        Hide
        Grant Ingersoll added a comment -

        Ran this last night:

        Testsuite: org.apache.lucene.index.Test2BTerms
        [junit] Tests run: 1, Failures: 0, Errors: 0, Time elapsed: 12,475.641 sec

        So, roughly 3.5 hours. Not at all unreasonable to run on a weekly basis.

        Show
        Grant Ingersoll added a comment - Ran this last night: Testsuite: org.apache.lucene.index.Test2BTerms [junit] Tests run: 1, Failures: 0, Errors: 0, Time elapsed: 12,475.641 sec So, roughly 3.5 hours. Not at all unreasonable to run on a weekly basis.
        Hide
        Grant Ingersoll added a comment -

        minor import fix, ready to go.

        Show
        Grant Ingersoll added a comment - minor import fix, ready to go.
        Hide
        Grant Ingersoll added a comment -

        Fixes as suggested by Robert. I'll likely commit at some point later today or tomorrow unless Hoss wants to discuss the weight thing more, in which case I'm happy to hold off.

        Show
        Grant Ingersoll added a comment - Fixes as suggested by Robert. I'll likely commit at some point later today or tomorrow unless Hoss wants to discuss the weight thing more, in which case I'm happy to hold off.
        Hide
        Grant Ingersoll added a comment -

        // please don't reorganize these into a wildcard!

        Hmm, I thought I disabled that in IntelliJ.

        Show
        Grant Ingersoll added a comment - // please don't reorganize these into a wildcard! Hmm, I thought I disabled that in IntelliJ.
        Hide
        Grant Ingersoll added a comment -

        I think in theory the weight thing sounds good, but in practice I think it would be hard to figure out the proper weights.

        Show
        Grant Ingersoll added a comment - I think in theory the weight thing sounds good, but in practice I think it would be hard to figure out the proper weights.
        Hide
        Robert Muir added a comment -

        rather then having "@Weekly" or "@Slow" (or even "@Nightly") annotations, we should just use an "@Weight(Integer)" annotation indicating how "heavy" a test is

        But I'm not sure I agree with this. Most tests have completely different runtime based on what configurations they get. Sometimes, rarely, we apply totally crazy parameters that make them go "nuts". Because of this we amortize the cost of checking expensive corner cases (like termIndexInterval=1, horrible params that cause a lot of merging, etc) across the different tests. I think this is good, it really finds corner case bugs. So if we had this weight annotation, i have no clue what we would assign it to anything.

        Because you take a test that does a lot of skipping/conjunctions, and if it gets memorycodec it might be take a terribly long time (since memorycodec doesn't support skipping), but otherwise it doesnt really care about termsindexinterval. On the other hand you take a test that is torturing the terms dictionary, and it doesnt care if you give it memorycodec or a shitty skipinterval, but you give it a crazy termsindexinterval and it spends a lot of time next'ing thru terms, etc.

        Because of this, all of the tests behave in totally different ways that you cant really assign a weight to, e.g. take a look at the history of test times
        for this test:

        https://builds.apache.org/view/G-L/view/Lucene/job/Lucene-Solr-tests-only-trunk/11631/testReport/org.apache.lucene.search/TestRegexpRandom2/history/

        Show
        Robert Muir added a comment - rather then having "@Weekly" or "@Slow" (or even "@Nightly") annotations, we should just use an "@Weight(Integer)" annotation indicating how "heavy" a test is But I'm not sure I agree with this. Most tests have completely different runtime based on what configurations they get. Sometimes, rarely, we apply totally crazy parameters that make them go "nuts". Because of this we amortize the cost of checking expensive corner cases (like termIndexInterval=1, horrible params that cause a lot of merging, etc) across the different tests. I think this is good, it really finds corner case bugs. So if we had this weight annotation, i have no clue what we would assign it to anything. Because you take a test that does a lot of skipping/conjunctions, and if it gets memorycodec it might be take a terribly long time (since memorycodec doesn't support skipping), but otherwise it doesnt really care about termsindexinterval. On the other hand you take a test that is torturing the terms dictionary, and it doesnt care if you give it memorycodec or a shitty skipinterval, but you give it a crazy termsindexinterval and it spends a lot of time next'ing thru terms, etc. Because of this, all of the tests behave in totally different ways that you cant really assign a weight to, e.g. take a look at the history of test times for this test: https://builds.apache.org/view/G-L/view/Lucene/job/Lucene-Solr-tests-only-trunk/11631/testReport/org.apache.lucene.search/TestRegexpRandom2/history/
        Hide
        Hoss Man added a comment -

        This reminds me of something i kept wanting to talk to Dawid about after his random testing talk at eurocon but didn't get a chance and then promptly forgot about.

        rather then having "@Weekly" or "@Slow" (or even "@Nightly") annotations, we should just use an "@Weight(Integer)" annotation indicating how "heavy" a test is (tests w/o an @Weight annotation would be treated as if they had an "@Weight(1)" annotation). The options for running tests would then let you specify the min/max weight for the tests you want to run, and instead of a "boolean isNightly()" there could be an "int maxTestWeight()" method that would factor into methods like "rarely()" and "usually()"

        So most tests would have no annotation, tests that currently use @Nightly would get @Weight(10) (or 5, or 100, or whatever) and tests like 2BTerms might get @Weight(10000).

        while the default "ant test" might use -Dtest.weight.min=1 -Dtest.weight.max=10 so developers and the jenkins post-commit build would run "most" tests, the "test only" jenkins build might be run with -Dtest.weight.min=1 -Dtest.weight.max=1 to only run the really fast test in a rapid fire, and a nightly jenkins run could use -Dtest.weight.min=1 -Dtest.weight.max=100 and a weekly (or monthly, or whatever) run might use -Dtest.weight.min=100 -Dtest.weight.max=100000 (ie: skip the quick tests and just run the heavy shit)

        Show
        Hoss Man added a comment - This reminds me of something i kept wanting to talk to Dawid about after his random testing talk at eurocon but didn't get a chance and then promptly forgot about. rather then having "@Weekly" or "@Slow" (or even "@Nightly") annotations, we should just use an "@Weight(Integer)" annotation indicating how "heavy" a test is (tests w/o an @Weight annotation would be treated as if they had an "@Weight(1)" annotation). The options for running tests would then let you specify the min/max weight for the tests you want to run, and instead of a "boolean isNightly()" there could be an "int maxTestWeight()" method that would factor into methods like "rarely()" and "usually()" So most tests would have no annotation, tests that currently use @Nightly would get @Weight(10) (or 5, or 100, or whatever) and tests like 2BTerms might get @Weight(10000). while the default "ant test" might use -Dtest.weight.min=1 -Dtest.weight.max=10 so developers and the jenkins post-commit build would run "most" tests, the "test only" jenkins build might be run with -Dtest.weight.min=1 -Dtest.weight.max=1 to only run the really fast test in a rapid fire, and a nightly jenkins run could use -Dtest.weight.min=1 -Dtest.weight.max=100 and a weekly (or monthly, or whatever) run might use -Dtest.weight.min=100 -Dtest.weight.max=100000 (ie: skip the quick tests and just run the heavy shit)
        Hide
        Uwe Schindler added a comment -

        +1, patch looks good!

        Show
        Uwe Schindler added a comment - +1, patch looks good!
        Hide
        Robert Muir added a comment -

        patch looks good to me.

        only one question about the patch, should Test2BTerms be both weekly and slow?
        Isn't it enough to just be slow? (then its only run if you ask for it).

        Separately I think weekly is useful and we can configure hudson to set this on sundays or something.

        the only other change i noticed was just purely cosmetic

        // please don't reorganize these into a wildcard!
        -import static org.apache.lucene.util.LuceneTestCase.TEST_ITER;
        -import static org.apache.lucene.util.LuceneTestCase.TEST_ITER_MIN;
        -import static org.apache.lucene.util.LuceneTestCase.TEST_METHOD;
        -import static org.apache.lucene.util.LuceneTestCase.TEST_SEED;
        -import static org.apache.lucene.util.LuceneTestCase.TEST_NIGHTLY;
        -import static org.apache.lucene.util.LuceneTestCase.VERBOSE;
        +import static org.apache.lucene.util.LuceneTestCase.*;
        
        Show
        Robert Muir added a comment - patch looks good to me. only one question about the patch, should Test2BTerms be both weekly and slow? Isn't it enough to just be slow? (then its only run if you ask for it). Separately I think weekly is useful and we can configure hudson to set this on sundays or something. the only other change i noticed was just purely cosmetic // please don't reorganize these into a wildcard! -import static org.apache.lucene.util.LuceneTestCase.TEST_ITER; -import static org.apache.lucene.util.LuceneTestCase.TEST_ITER_MIN; -import static org.apache.lucene.util.LuceneTestCase.TEST_METHOD; -import static org.apache.lucene.util.LuceneTestCase.TEST_SEED; -import static org.apache.lucene.util.LuceneTestCase.TEST_NIGHTLY; -import static org.apache.lucene.util.LuceneTestCase.VERBOSE; +import static org.apache.lucene.util.LuceneTestCase.*;
        Hide
        Grant Ingersoll added a comment -

        Here's a first go at this. Added @Weekly and @Slow. Added the corresponding properties to common-build.xml. Added ability to control test heapsize to common-build. Marked Test2BTerms as @Weekly and @Slow. Also makred it to not use poor codecs. Refactored the test runner a bit to share common code when removing annotated tests.

        Still testing whether this works, but thought I would put it up for feedback from those who have big machines.

        Show
        Grant Ingersoll added a comment - Here's a first go at this. Added @Weekly and @Slow. Added the corresponding properties to common-build.xml. Added ability to control test heapsize to common-build. Marked Test2BTerms as @Weekly and @Slow. Also makred it to not use poor codecs. Refactored the test runner a bit to share common code when removing annotated tests. Still testing whether this works, but thought I would put it up for feedback from those who have big machines.
        Hide
        Robert Muir added a comment -

        we probably need to make sure the ram we give to junit in common-build.xml is parameterized too ( i think we hardcode 512MB )

        at least so you could say ant test -Dtests.ram=8GB -Drunreallyslowtests or whatever

        Show
        Robert Muir added a comment - we probably need to make sure the ram we give to junit in common-build.xml is parameterized too ( i think we hardcode 512MB ) at least so you could say ant test -Dtests.ram=8GB -Drunreallyslowtests or whatever

          People

          • Assignee:
            Grant Ingersoll
            Reporter:
            Grant Ingersoll
          • Votes:
            0 Vote for this issue
            Watchers:
            1 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development