castor

Refactor old tests TC30 and TC31

Details

  • Type: Sub-task Sub-task
  • Status: Closed Closed
  • Priority: Minor Minor
  • Resolution: Fixed
  • Affects Version/s: 1.2-SNAPSHOT
  • Fix Version/s: 1.3 rc1
  • Component/s: JDO tests
  • Labels:
    None
  • Number of attachments :
    9
  1. Text File
    patch20080505-test30-03.txt
    05/May/08 1:22 PM
    70 kB
    Udai Gupta
  2. Text File
    patch20080505-test30-test31.txt
    05/May/08 2:35 PM
    139 kB
    Udai Gupta
  3. File
    patch20080506_test30_test31-01.diff
    06/May/08 5:29 AM
    72 kB
    Udai Gupta
  4. File
    patch20080507-C2377-running.diff
    06/May/08 11:16 PM
    74 kB
    Udai Gupta
  5. Text File
    patch-C2377-20080506-02.txt
    06/May/08 4:45 PM
    68 kB
    Ralf Joachim
  6. Text File
    patch-C2377-20080512.txt
    12/May/08 4:50 PM
    73 kB
    Ralf Joachim
  7. Text File
    patch-TC30-20080505.txt
    05/May/08 12:02 AM
    123 kB
    Udai Gupta
  8. Text File
    patch-test30-20080505-01.txt
    05/May/08 6:01 AM
    72 kB
    Udai Gupta
  9. Text File
    patch-test30-20080505-02.txt
    05/May/08 6:26 AM
    72 kB
    Udai Gupta

Issue Links

is related to

Improvement - An improvement or enhancement to an existing feature or task. CASTOR-2396 Refactor OQL query engine during GSoC 2008

  • Major - Major loss of function.
  • Closed - The issue is considered finished, the resolution is correct. Issues which are not closed can be reopened.

Activity

Ascending order - Click to sort in descending order
Hide
Permalink
Udai Gupta added a comment -

I have attached the patch for the Refactorization TC30 into test30. Here I have created a separate test method for each query.

Show
Udai Gupta added a comment - I have attached the patch for the Refactorization TC30 into test30. Here I have created a separate test method for each query.
Hide
Permalink
Werner Guttmann added a comment -

Udai, I don't think there's any value in overloading the suite() method in this test case. JUnit 3.x has a default convention where any method will be added to the suite if it has a prefix of 'test'. As we have followed that convention throughout, please drop this method.

Show
Werner Guttmann added a comment - Udai, I don't think there's any value in overloading the suite() method in this test case. JUnit 3.x has a default convention where any method will be added to the suite if it has a prefix of 'test'. As we have followed that convention throughout, please drop this method.
Hide
Permalink
Werner Guttmann added a comment -

A few more observations:

a) Using code statements in setUp() like this

LOG.info("Test30 Started");

could be slightly mis-leading, as the methods setUp() and tearDown() will be executed before and after each test case (as opposed to once per test class).

b) the populateDatabase*() methods should imho not be included in the test suite (if we decide to keep a suite method, which in my opinion is completely redundant).

c) Shouldn't the populateDatabase*() methods be included in the setUp() method, so that each test has a clean database before it is being executed ?

Show
Werner Guttmann added a comment - A few more observations: a) Using code statements in setUp() like this LOG.info( "Test30 Started" ); could be slightly mis-leading, as the methods setUp() and tearDown() will be executed before and after each test case (as opposed to once per test class). b) the populateDatabase*() methods should imho not be included in the test suite (if we decide to keep a suite method, which in my opinion is completely redundant). c) Shouldn't the populateDatabase*() methods be included in the setUp() method, so that each test has a clean database before it is being executed ?
Hide
Permalink
Ralf Joachim added a comment -

I hope to be able to review your patch in detail taking Werner's comments into account later on today. ATM I only recognized that some changes are contained twice in the patch. In addition you seam to have also moved entities not used by TC30 but other TC3x tests.

Show
Ralf Joachim added a comment - I hope to be able to review your patch in detail taking Werner's comments into account later on today. ATM I only recognized that some changes are contained twice in the patch. In addition you seam to have also moved entities not used by TC30 but other TC3x tests.
Hide
Permalink
Udai Gupta added a comment -

Thanks for your comments ....I have modified accordingly ....I hope this time it will be fine.

Show
Udai Gupta added a comment - Thanks for your comments ....I have modified accordingly ....I hope this time it will be fine.
Hide
Permalink
Werner Guttmann added a comment -

Yes, much better. But there was no need to actually remove the populateDatabase*() methods and move the code into the setUp() method. Just leave those methods as is, and simply call them from the setUp() method.

Show
Werner Guttmann added a comment - Yes, much better. But there was no need to actually remove the populateDatabase*() methods and move the code into the setUp() method. Just leave those methods as is, and simply call them from the setUp() method.
Hide
Permalink
Udai Gupta added a comment -

done!!

Show
Udai Gupta added a comment - done!!
Hide
Permalink
Werner Guttmann added a comment -

Great. Please leave the call to 

_db = getJDOManager(DBNAME).getDatabase();

in the setUp()method, though (as that's where it belongs to ...) .. .

Show
Werner Guttmann added a comment - Great. Please leave the call to _db = getJDOManager(DBNAME).getDatabase(); in the setUp()method, though (as that's where it belongs to ...) .. .
Hide
Permalink
Ralf Joachim added a comment -

Updated patch with a bunch of small improvements to test30. According to the tests i extracted 2 tests of test30 into a seperate test class but most was about code formating. Having said that I had not enough time to test them.

Udai, could you please test my changes and review test31 yourself with respect to the changes I made to test30.

Show
Ralf Joachim added a comment - Updated patch with a bunch of small improvements to test30. According to the tests i extracted 2 tests of test30 into a seperate test class but most was about code formating. Having said that I had not enough time to test them. Udai, could you please test my changes and review test31 yourself with respect to the changes I made to test30.
Hide
Permalink
Udai Gupta added a comment -

Thanks Ralf , I will take care of all this next time when I will code.....There was some minor problems in the patch of tests you provided in context of running them....I fixed those ...now its perfect....I will modify the TC31 accordingly

Show
Udai Gupta added a comment - Thanks Ralf , I will take care of all this next time when I will code.....There was some minor problems in the patch of tests you provided in context of running them....I fixed those ...now its perfect....I will modify the TC31 accordingly
Hide
Permalink
Ralf Joachim added a comment -

Again removed test30_call table from ddl script and also had to do some code formatting at test31. Of what I recall there where class variables that where only needed by one method and did not conform to our naming conventions for class variables and a lot of lines with wrong indention. Also one of the table names did not start with test31. In addition some of the tables got not reset before test execution.

I have fixed all this myself and added release notes so that the patch now is ready to commit.

Having said that all tests already run without failures against mysql.

Show
Ralf Joachim added a comment - Again removed test30_call table from ddl script and also had to do some code formatting at test31. Of what I recall there where class variables that where only needed by one method and did not conform to our naming conventions for class variables and a lot of lines with wrong indention. Also one of the table names did not start with test31. In addition some of the tables got not reset before test execution. I have fixed all this myself and added release notes so that the patch now is ready to commit. Having said that all tests already run without failures against mysql.

People

  • Assignee:
    Ralf Joachim
    Reporter:
    Udai Gupta
Vote (0)
Watch (0)

Dates

  • Created:
    Updated:
    Resolved: