[ 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-unsubscr...@lucene.apache.org For additional commands, e-mail: dev-h...@lucene.apache.org