Need to Reactor US Calendar Test classes and add missing tests (2011,2012)

It's not necessary to separate different exchange into different classes.
On the other hand, it would be interesting to make the code easier to read. At the moment there are several dates added to some exchanges and/or removed from other exchanges. This make the code to be confused. It would be better to repeat blocks but keep one exchange totally separated from each other in each test case.

I've done a code review and sorted out some issues but it's still necessary to organize better the test cases, separating different exchanges into different test cases.

No need to have separate files for separate tests.

Can this issue be marked as RESOLVED?

Thanks

Yes, we can mark it as RESOLVED.
Note: There were existing test cases which test holidays for all US calendars by year but QL097 test cases test one calendar for all the years(but only upto 2005). I did not remove existing test cases because they test holidays upto year 2012.

Yes. It can be marked as resolved. But still it will be a good idea to separate tests for different exchanges into different files. That improves readability and makes the tests to be manageable. This is not a requirement specific to this test, but to almost all of the calendar tests.

Hi Renjith,

My only concern is regarding the possibility of different class names when we compare against QuantLib/C++. As we have many test cases, we would potentially causing many differences. I explain the reason below:

At certain point I plan to have a tool which is intended to compare our class hierarchy, method names and field names against QuantLib/C++. The process consists of generating XML documentation of JQuantLib and QuantLib using Doxygen. A tool in Python will be responsible for comparing both XML files, hiding differences we know can be hidden and telling us other differences we are not expecting to see.

Test cases can be moved to other files... I dont think there's any thourble... but it would probably imply on different class names when we compare against QuantLib/c++ in future, which would generate dozens of warnings.

I havent stopped by to analyze the problem so, maybe it's possible to improve the way it is without generating differences.

Please let me know what you think about it and let me know about your plans to improve it.

Note: things can be improved! there's no need to strictly follow exactly everything which was done in QuantLib/C++. On the other hand, we should do our best to keep our API as close as possible, whenever it is possible and convenient.

Hi Richard,

My understanding was that we will be comparing only the actual classes and not the test classes. When we are writing a comparison tool we could easily ignore all classes under the path 'test' .

The idea of separating tests came up due to the fact that when the test classes are over 200-300 lines of code, it is cumbersome to manage it. There is nothing preventing us from keeping all the test cases in one file but i have seen from my experience that it is very difficult to manage them in future. We can name the tests like USCalendarSettlementTest, USCalendarNasdaqTest etc. This is just a suggestion for increasing the maintainability of code.

To give uniformity we can keep the calendar classes as it is and for the ones with different subdivisions, we can keep different tests which will follow an identifiable naming pattern. This will enable us to compare the tests too if needed in future.

Please let me know your opinion regarding this. If it is too much of work, we can leave the tests as it is (in single test class) and then if needed in future we can divide them in to separate files. This is not a show stopper and so the priority can be low for it.

Hi Renjith,

Agreed.
For your information, I've created a ticket based on your idea:
http://bugs.jquantlib.org/view.php?id=400 (parent)
http://bugs.jquantlib.org/view.php?id=402

So...
Would you like to refactor the tests then?

kicking to v0.1.4

Request for status update.

This issue looks abandon.

Abandoned.