You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@lucene.apache.org by "Shai Erera (JIRA)" <ji...@apache.org> on 2010/07/27 06:14:15 UTC

[jira] Created: (LUCENE-2570) Some improvements to _TestUtil and its usage

Some improvements to _TestUtil and its usage
--------------------------------------------

                 Key: LUCENE-2570
                 URL: https://issues.apache.org/jira/browse/LUCENE-2570
             Project: Lucene - Java
          Issue Type: Test
            Reporter: Shai Erera
            Assignee: Shai Erera
            Priority: Minor
             Fix For: 4.0


I've started this issue because I've noticed that _TestUtil.getRandomMultiplier() is called from many loops' condition check, sometimes hundreds and thousands of times. Each time it does Integer.parseInt after calling System.getProperty. This really can become a constant IMO, either in LuceneTestCase(J4) or _TestUtil, as it's not expected to change while tests are running ...

I then reviewed the class and spotted some more things that I think can be fixed/improved:
# getTestCodec() can become a constant as well
# arrayToString is marked deprecated. I've checked an no one calls them, so I'll delete them. This is a 4.0 code branch + a test-only class. No need to deprecate anything.
# getTempDir calls new Random(), instead of newRandom() in LuceneTestCaseJ4, which means that if something fails, we won't know the random seed used ...
#* In that regard, we might want to output all the classes that obtained a static seed in reportAdditionalFailures(), instead of just the class that ran the test.
# rmDir(String) can be removed IMO, and leave only rmDir(File)
# I suggest we include some recursion in rmDir(File) to handle the deletion of nested directories.
#* Also, it does not check whether the dir deletion itself succeeds (but it does so for the files). This can bite us on Windows, if some test did not close things properly.

I'll work out a patch.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org


[jira] Commented: (LUCENE-2570) Some improvements to _TestUtil and its usage

Posted by "Shai Erera (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/LUCENE-2570?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12892693#action_12892693 ] 

Shai Erera commented on LUCENE-2570:
------------------------------------

I was thinking of the fix to getTempDir that I proposed. If _TestUtil should obtain such static random, then it (1) needs to extend LuceneTestCaseJ4 and (2) will affect all the classes that call this method.

I'm not sure I like having _TestUtil extending LTCJ4, and not sure that particular random can affect any test failure ... so perhaps I'll leave that fix outside?

> Some improvements to _TestUtil and its usage
> --------------------------------------------
>
>                 Key: LUCENE-2570
>                 URL: https://issues.apache.org/jira/browse/LUCENE-2570
>             Project: Lucene - Java
>          Issue Type: Test
>            Reporter: Shai Erera
>            Assignee: Shai Erera
>            Priority: Minor
>             Fix For: 4.0
>
>
> I've started this issue because I've noticed that _TestUtil.getRandomMultiplier() is called from many loops' condition check, sometimes hundreds and thousands of times. Each time it does Integer.parseInt after calling System.getProperty. This really can become a constant IMO, either in LuceneTestCase(J4) or _TestUtil, as it's not expected to change while tests are running ...
> I then reviewed the class and spotted some more things that I think can be fixed/improved:
> # getTestCodec() can become a constant as well
> # arrayToString is marked deprecated. I've checked an no one calls them, so I'll delete them. This is a 4.0 code branch + a test-only class. No need to deprecate anything.
> # getTempDir calls new Random(), instead of newRandom() in LuceneTestCaseJ4, which means that if something fails, we won't know the random seed used ...
> #* In that regard, we might want to output all the classes that obtained a static seed in reportAdditionalFailures(), instead of just the class that ran the test.
> # rmDir(String) can be removed IMO, and leave only rmDir(File)
> # I suggest we include some recursion in rmDir(File) to handle the deletion of nested directories.
> #* Also, it does not check whether the dir deletion itself succeeds (but it does so for the files). This can bite us on Windows, if some test did not close things properly.
> I'll work out a patch.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org


[jira] Commented: (LUCENE-2570) Some improvements to _TestUtil and its usage

Posted by "Shai Erera (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/LUCENE-2570?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12892711#action_12892711 ] 

Shai Erera commented on LUCENE-2570:
------------------------------------

I don't think that removing the class is a good idea. Some methods like rmDir can exist on the base test classes, however the class also contains large arrays blockStarts/Ends and it will just clatter the base test class, I think. What do you think?

bq. Another option, is if _TestUtil has methods that need a random, make it a required argument? 

I had thought about it, but that method is called from several places in the code, and the randomness seems very insignificant. It just adds a random number to the directory name the caller requested. If it is in order to prevent collisions, then the code isn't safe because there is a chance the same number will be generated twice ...

> Some improvements to _TestUtil and its usage
> --------------------------------------------
>
>                 Key: LUCENE-2570
>                 URL: https://issues.apache.org/jira/browse/LUCENE-2570
>             Project: Lucene - Java
>          Issue Type: Test
>            Reporter: Shai Erera
>            Assignee: Shai Erera
>            Priority: Minor
>             Fix For: 4.0
>
>
> I've started this issue because I've noticed that _TestUtil.getRandomMultiplier() is called from many loops' condition check, sometimes hundreds and thousands of times. Each time it does Integer.parseInt after calling System.getProperty. This really can become a constant IMO, either in LuceneTestCase(J4) or _TestUtil, as it's not expected to change while tests are running ...
> I then reviewed the class and spotted some more things that I think can be fixed/improved:
> # getTestCodec() can become a constant as well
> # arrayToString is marked deprecated. I've checked an no one calls them, so I'll delete them. This is a 4.0 code branch + a test-only class. No need to deprecate anything.
> # getTempDir calls new Random(), instead of newRandom() in LuceneTestCaseJ4, which means that if something fails, we won't know the random seed used ...
> #* In that regard, we might want to output all the classes that obtained a static seed in reportAdditionalFailures(), instead of just the class that ran the test.
> # rmDir(String) can be removed IMO, and leave only rmDir(File)
> # I suggest we include some recursion in rmDir(File) to handle the deletion of nested directories.
> #* Also, it does not check whether the dir deletion itself succeeds (but it does so for the files). This can bite us on Windows, if some test did not close things properly.
> I'll work out a patch.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org


[jira] Commented: (LUCENE-2570) Some improvements to _TestUtil and its usage

Posted by "Robert Muir (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/LUCENE-2570?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12892700#action_12892700 ] 

Robert Muir commented on LUCENE-2570:
-------------------------------------

Another option, is if _TestUtil has methods that need a random, make it a required argument?

> Some improvements to _TestUtil and its usage
> --------------------------------------------
>
>                 Key: LUCENE-2570
>                 URL: https://issues.apache.org/jira/browse/LUCENE-2570
>             Project: Lucene - Java
>          Issue Type: Test
>            Reporter: Shai Erera
>            Assignee: Shai Erera
>            Priority: Minor
>             Fix For: 4.0
>
>
> I've started this issue because I've noticed that _TestUtil.getRandomMultiplier() is called from many loops' condition check, sometimes hundreds and thousands of times. Each time it does Integer.parseInt after calling System.getProperty. This really can become a constant IMO, either in LuceneTestCase(J4) or _TestUtil, as it's not expected to change while tests are running ...
> I then reviewed the class and spotted some more things that I think can be fixed/improved:
> # getTestCodec() can become a constant as well
> # arrayToString is marked deprecated. I've checked an no one calls them, so I'll delete them. This is a 4.0 code branch + a test-only class. No need to deprecate anything.
> # getTempDir calls new Random(), instead of newRandom() in LuceneTestCaseJ4, which means that if something fails, we won't know the random seed used ...
> #* In that regard, we might want to output all the classes that obtained a static seed in reportAdditionalFailures(), instead of just the class that ran the test.
> # rmDir(String) can be removed IMO, and leave only rmDir(File)
> # I suggest we include some recursion in rmDir(File) to handle the deletion of nested directories.
> #* Also, it does not check whether the dir deletion itself succeeds (but it does so for the files). This can bite us on Windows, if some test did not close things properly.
> I'll work out a patch.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org


[jira] Commented: (LUCENE-2570) Some improvements to _TestUtil and its usage

Posted by "Robert Muir (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/LUCENE-2570?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12892716#action_12892716 ] 

Robert Muir commented on LUCENE-2570:
-------------------------------------

the only comment i have is exposing TEST_CODEC as public... can it be private? 

I don't think its very useful since it could be a value like "random" (and is by default!) which isnt actually a valid codec, its basically just internal to LuceneTestCase[J4]

> Some improvements to _TestUtil and its usage
> --------------------------------------------
>
>                 Key: LUCENE-2570
>                 URL: https://issues.apache.org/jira/browse/LUCENE-2570
>             Project: Lucene - Java
>          Issue Type: Test
>            Reporter: Shai Erera
>            Assignee: Shai Erera
>            Priority: Minor
>             Fix For: 4.0
>
>         Attachments: LUCENE-2570.patch
>
>
> I've started this issue because I've noticed that _TestUtil.getRandomMultiplier() is called from many loops' condition check, sometimes hundreds and thousands of times. Each time it does Integer.parseInt after calling System.getProperty. This really can become a constant IMO, either in LuceneTestCase(J4) or _TestUtil, as it's not expected to change while tests are running ...
> I then reviewed the class and spotted some more things that I think can be fixed/improved:
> # getTestCodec() can become a constant as well
> # arrayToString is marked deprecated. I've checked an no one calls them, so I'll delete them. This is a 4.0 code branch + a test-only class. No need to deprecate anything.
> # getTempDir calls new Random(), instead of newRandom() in LuceneTestCaseJ4, which means that if something fails, we won't know the random seed used ...
> #* In that regard, we might want to output all the classes that obtained a static seed in reportAdditionalFailures(), instead of just the class that ran the test.
> # rmDir(String) can be removed IMO, and leave only rmDir(File)
> # I suggest we include some recursion in rmDir(File) to handle the deletion of nested directories.
> #* Also, it does not check whether the dir deletion itself succeeds (but it does so for the files). This can bite us on Windows, if some test did not close things properly.
> I'll work out a patch.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org


[jira] Commented: (LUCENE-2570) Some improvements to _TestUtil and its usage

Posted by "Uwe Schindler (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/LUCENE-2570?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12892694#action_12892694 ] 

Uwe Schindler commented on LUCENE-2570:
---------------------------------------

We can remove the whole class and move the code to LTC(J4)?

> Some improvements to _TestUtil and its usage
> --------------------------------------------
>
>                 Key: LUCENE-2570
>                 URL: https://issues.apache.org/jira/browse/LUCENE-2570
>             Project: Lucene - Java
>          Issue Type: Test
>            Reporter: Shai Erera
>            Assignee: Shai Erera
>            Priority: Minor
>             Fix For: 4.0
>
>
> I've started this issue because I've noticed that _TestUtil.getRandomMultiplier() is called from many loops' condition check, sometimes hundreds and thousands of times. Each time it does Integer.parseInt after calling System.getProperty. This really can become a constant IMO, either in LuceneTestCase(J4) or _TestUtil, as it's not expected to change while tests are running ...
> I then reviewed the class and spotted some more things that I think can be fixed/improved:
> # getTestCodec() can become a constant as well
> # arrayToString is marked deprecated. I've checked an no one calls them, so I'll delete them. This is a 4.0 code branch + a test-only class. No need to deprecate anything.
> # getTempDir calls new Random(), instead of newRandom() in LuceneTestCaseJ4, which means that if something fails, we won't know the random seed used ...
> #* In that regard, we might want to output all the classes that obtained a static seed in reportAdditionalFailures(), instead of just the class that ran the test.
> # rmDir(String) can be removed IMO, and leave only rmDir(File)
> # I suggest we include some recursion in rmDir(File) to handle the deletion of nested directories.
> #* Also, it does not check whether the dir deletion itself succeeds (but it does so for the files). This can bite us on Windows, if some test did not close things properly.
> I'll work out a patch.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org


[jira] Updated: (LUCENE-2570) Some improvements to _TestUtil and its usage

Posted by "Shai Erera (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/LUCENE-2570?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Shai Erera updated LUCENE-2570:
-------------------------------

    Attachment: LUCENE-2570.patch

Most of the patch is about replacing the calls to getRandomMultiplier w/ the static constant reference. All tests pass, except the one that is fixed in LUCENE-2568 (but not committed yet). Unless you have more comments, I'd like to commit this shortly.

> Some improvements to _TestUtil and its usage
> --------------------------------------------
>
>                 Key: LUCENE-2570
>                 URL: https://issues.apache.org/jira/browse/LUCENE-2570
>             Project: Lucene - Java
>          Issue Type: Test
>            Reporter: Shai Erera
>            Assignee: Shai Erera
>            Priority: Minor
>             Fix For: 4.0
>
>         Attachments: LUCENE-2570.patch
>
>
> I've started this issue because I've noticed that _TestUtil.getRandomMultiplier() is called from many loops' condition check, sometimes hundreds and thousands of times. Each time it does Integer.parseInt after calling System.getProperty. This really can become a constant IMO, either in LuceneTestCase(J4) or _TestUtil, as it's not expected to change while tests are running ...
> I then reviewed the class and spotted some more things that I think can be fixed/improved:
> # getTestCodec() can become a constant as well
> # arrayToString is marked deprecated. I've checked an no one calls them, so I'll delete them. This is a 4.0 code branch + a test-only class. No need to deprecate anything.
> # getTempDir calls new Random(), instead of newRandom() in LuceneTestCaseJ4, which means that if something fails, we won't know the random seed used ...
> #* In that regard, we might want to output all the classes that obtained a static seed in reportAdditionalFailures(), instead of just the class that ran the test.
> # rmDir(String) can be removed IMO, and leave only rmDir(File)
> # I suggest we include some recursion in rmDir(File) to handle the deletion of nested directories.
> #* Also, it does not check whether the dir deletion itself succeeds (but it does so for the files). This can bite us on Windows, if some test did not close things properly.
> I'll work out a patch.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org


[jira] Commented: (LUCENE-2570) Some improvements to _TestUtil and its usage

Posted by "Shai Erera (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/LUCENE-2570?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12892722#action_12892722 ] 

Shai Erera commented on LUCENE-2570:
------------------------------------

I can make it private, but then I'll need to repeat the declaration line in both LTC and LTCJ4. Currently, LTC just references the one in LTCJ4. If that matters, I can make it package-private?

> Some improvements to _TestUtil and its usage
> --------------------------------------------
>
>                 Key: LUCENE-2570
>                 URL: https://issues.apache.org/jira/browse/LUCENE-2570
>             Project: Lucene - Java
>          Issue Type: Test
>            Reporter: Shai Erera
>            Assignee: Shai Erera
>            Priority: Minor
>             Fix For: 4.0
>
>         Attachments: LUCENE-2570.patch
>
>
> I've started this issue because I've noticed that _TestUtil.getRandomMultiplier() is called from many loops' condition check, sometimes hundreds and thousands of times. Each time it does Integer.parseInt after calling System.getProperty. This really can become a constant IMO, either in LuceneTestCase(J4) or _TestUtil, as it's not expected to change while tests are running ...
> I then reviewed the class and spotted some more things that I think can be fixed/improved:
> # getTestCodec() can become a constant as well
> # arrayToString is marked deprecated. I've checked an no one calls them, so I'll delete them. This is a 4.0 code branch + a test-only class. No need to deprecate anything.
> # getTempDir calls new Random(), instead of newRandom() in LuceneTestCaseJ4, which means that if something fails, we won't know the random seed used ...
> #* In that regard, we might want to output all the classes that obtained a static seed in reportAdditionalFailures(), instead of just the class that ran the test.
> # rmDir(String) can be removed IMO, and leave only rmDir(File)
> # I suggest we include some recursion in rmDir(File) to handle the deletion of nested directories.
> #* Also, it does not check whether the dir deletion itself succeeds (but it does so for the files). This can bite us on Windows, if some test did not close things properly.
> I'll work out a patch.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org


[jira] Commented: (LUCENE-2570) Some improvements to _TestUtil and its usage

Posted by "Shai Erera (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/LUCENE-2570?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12892729#action_12892729 ] 

Shai Erera commented on LUCENE-2570:
------------------------------------

ok will do, and commit.

> Some improvements to _TestUtil and its usage
> --------------------------------------------
>
>                 Key: LUCENE-2570
>                 URL: https://issues.apache.org/jira/browse/LUCENE-2570
>             Project: Lucene - Java
>          Issue Type: Test
>            Reporter: Shai Erera
>            Assignee: Shai Erera
>            Priority: Minor
>             Fix For: 4.0
>
>         Attachments: LUCENE-2570.patch
>
>
> I've started this issue because I've noticed that _TestUtil.getRandomMultiplier() is called from many loops' condition check, sometimes hundreds and thousands of times. Each time it does Integer.parseInt after calling System.getProperty. This really can become a constant IMO, either in LuceneTestCase(J4) or _TestUtil, as it's not expected to change while tests are running ...
> I then reviewed the class and spotted some more things that I think can be fixed/improved:
> # getTestCodec() can become a constant as well
> # arrayToString is marked deprecated. I've checked an no one calls them, so I'll delete them. This is a 4.0 code branch + a test-only class. No need to deprecate anything.
> # getTempDir calls new Random(), instead of newRandom() in LuceneTestCaseJ4, which means that if something fails, we won't know the random seed used ...
> #* In that regard, we might want to output all the classes that obtained a static seed in reportAdditionalFailures(), instead of just the class that ran the test.
> # rmDir(String) can be removed IMO, and leave only rmDir(File)
> # I suggest we include some recursion in rmDir(File) to handle the deletion of nested directories.
> #* Also, it does not check whether the dir deletion itself succeeds (but it does so for the files). This can bite us on Windows, if some test did not close things properly.
> I'll work out a patch.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org


[jira] Resolved: (LUCENE-2570) Some improvements to _TestUtil and its usage

Posted by "Shai Erera (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/LUCENE-2570?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Shai Erera resolved LUCENE-2570.
--------------------------------

    Fix Version/s: 3.1
       Resolution: Fixed

Committed revision 979646 (trunk).
Committed revision 979720 (3x).

> Some improvements to _TestUtil and its usage
> --------------------------------------------
>
>                 Key: LUCENE-2570
>                 URL: https://issues.apache.org/jira/browse/LUCENE-2570
>             Project: Lucene - Java
>          Issue Type: Test
>            Reporter: Shai Erera
>            Assignee: Shai Erera
>            Priority: Minor
>             Fix For: 3.1, 4.0
>
>         Attachments: LUCENE-2570.patch
>
>
> I've started this issue because I've noticed that _TestUtil.getRandomMultiplier() is called from many loops' condition check, sometimes hundreds and thousands of times. Each time it does Integer.parseInt after calling System.getProperty. This really can become a constant IMO, either in LuceneTestCase(J4) or _TestUtil, as it's not expected to change while tests are running ...
> I then reviewed the class and spotted some more things that I think can be fixed/improved:
> # getTestCodec() can become a constant as well
> # arrayToString is marked deprecated. I've checked an no one calls them, so I'll delete them. This is a 4.0 code branch + a test-only class. No need to deprecate anything.
> # getTempDir calls new Random(), instead of newRandom() in LuceneTestCaseJ4, which means that if something fails, we won't know the random seed used ...
> #* In that regard, we might want to output all the classes that obtained a static seed in reportAdditionalFailures(), instead of just the class that ran the test.
> # rmDir(String) can be removed IMO, and leave only rmDir(File)
> # I suggest we include some recursion in rmDir(File) to handle the deletion of nested directories.
> #* Also, it does not check whether the dir deletion itself succeeds (but it does so for the files). This can bite us on Windows, if some test did not close things properly.
> I'll work out a patch.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org


[jira] Commented: (LUCENE-2570) Some improvements to _TestUtil and its usage

Posted by "Robert Muir (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/LUCENE-2570?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12892725#action_12892725 ] 

Robert Muir commented on LUCENE-2570:
-------------------------------------

package-private works... i just wanted to prevent confusion since i don't think its very useful outside of LuceneTestCase[J4] 

> Some improvements to _TestUtil and its usage
> --------------------------------------------
>
>                 Key: LUCENE-2570
>                 URL: https://issues.apache.org/jira/browse/LUCENE-2570
>             Project: Lucene - Java
>          Issue Type: Test
>            Reporter: Shai Erera
>            Assignee: Shai Erera
>            Priority: Minor
>             Fix For: 4.0
>
>         Attachments: LUCENE-2570.patch
>
>
> I've started this issue because I've noticed that _TestUtil.getRandomMultiplier() is called from many loops' condition check, sometimes hundreds and thousands of times. Each time it does Integer.parseInt after calling System.getProperty. This really can become a constant IMO, either in LuceneTestCase(J4) or _TestUtil, as it's not expected to change while tests are running ...
> I then reviewed the class and spotted some more things that I think can be fixed/improved:
> # getTestCodec() can become a constant as well
> # arrayToString is marked deprecated. I've checked an no one calls them, so I'll delete them. This is a 4.0 code branch + a test-only class. No need to deprecate anything.
> # getTempDir calls new Random(), instead of newRandom() in LuceneTestCaseJ4, which means that if something fails, we won't know the random seed used ...
> #* In that regard, we might want to output all the classes that obtained a static seed in reportAdditionalFailures(), instead of just the class that ran the test.
> # rmDir(String) can be removed IMO, and leave only rmDir(File)
> # I suggest we include some recursion in rmDir(File) to handle the deletion of nested directories.
> #* Also, it does not check whether the dir deletion itself succeeds (but it does so for the files). This can bite us on Windows, if some test did not close things properly.
> I'll work out a patch.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org


[jira] Commented: (LUCENE-2570) Some improvements to _TestUtil and its usage

Posted by "Robert Muir (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/LUCENE-2570?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12892691#action_12892691 ] 

Robert Muir commented on LUCENE-2570:
-------------------------------------

bq. In that regard, we might want to output all the classes that obtained a static seed in reportAdditionalFailures(), instead of just the class that ran the test.

I think that one might be confusing? because one test class can't really affect another. I like all your other suggestions though!

> Some improvements to _TestUtil and its usage
> --------------------------------------------
>
>                 Key: LUCENE-2570
>                 URL: https://issues.apache.org/jira/browse/LUCENE-2570
>             Project: Lucene - Java
>          Issue Type: Test
>            Reporter: Shai Erera
>            Assignee: Shai Erera
>            Priority: Minor
>             Fix For: 4.0
>
>
> I've started this issue because I've noticed that _TestUtil.getRandomMultiplier() is called from many loops' condition check, sometimes hundreds and thousands of times. Each time it does Integer.parseInt after calling System.getProperty. This really can become a constant IMO, either in LuceneTestCase(J4) or _TestUtil, as it's not expected to change while tests are running ...
> I then reviewed the class and spotted some more things that I think can be fixed/improved:
> # getTestCodec() can become a constant as well
> # arrayToString is marked deprecated. I've checked an no one calls them, so I'll delete them. This is a 4.0 code branch + a test-only class. No need to deprecate anything.
> # getTempDir calls new Random(), instead of newRandom() in LuceneTestCaseJ4, which means that if something fails, we won't know the random seed used ...
> #* In that regard, we might want to output all the classes that obtained a static seed in reportAdditionalFailures(), instead of just the class that ran the test.
> # rmDir(String) can be removed IMO, and leave only rmDir(File)
> # I suggest we include some recursion in rmDir(File) to handle the deletion of nested directories.
> #* Also, it does not check whether the dir deletion itself succeeds (but it does so for the files). This can bite us on Windows, if some test did not close things properly.
> I'll work out a patch.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org