Re: [Rails-devel] Automated testing for Rails

[brett l. <bre...@gm...>]

On Tue, Mar 23, 2010 at 3:17 PM, Stefan Frey <ste...@we...> wrote:
> Brett,
> see my comments below.
> Stefan
>
>> Excellent. We've needed this for a while, and this is a good start.
>
> Thanks. I am happy to have that too, as it now easier to refactor classes.
>>
>> This is fine as a starting point, but it's not how most traditional
>> automated testing works.
>
> Yes and no. It is automated testing, but obviously it is not (traditiional)
> unit testing, but belongs to the class of integration or system tests. But
> all are equally important.
>

Agreed. I phrased it badly, but yes, we're saying the same things.

>>
>> One goal that I hope we work toward is building on this foundation by
>> developing unit and regression tests for each object that doesn't
>> require a save file.
>>
>> Ideally, we should be able to build out mock objects of most things.
>> If we can't, that might be an indicator that refactoring is needed. If
>> nothing else, it should spawn some javadoc about why the object is
>> difficult/impossible to test.
>>
>
> The major issue I see why people are reluctant to write unit tests for
> existing code in Rails currently, is that it is much work to do with no
> immediate reward. This is very similar to ask someone for writing
> documentation for existing code.
>

Yup.  Mostly I was pointing out that we have a starting point. We can
build on that incrementally.

I didn't mean to make it sound like you should do all of the work. It
was more that, now we can consider building tests as we touch old code
and/or as we write new code.

These things are always much easier if we tackle them a little bit at
a time, as we work on other things.

> For the integration tests above I could leverage on existing mechanisms:
> The action stack (already used for the save files) to replay games and the
> ReportBuffer, that collects the game report. Thus easy to do.
>
> My goal for working on this project is to use it for playing 18xx: That is the
> basis for all my contributions so far: We usually play with a real map =>
> nomap Mode. I want to be able to make corrections on the fly => correction
> actions. I like 1889 => 1889!
>
> And why the tests now? My next focus will be 1870. 1870 adds some weird stuff
> to the stockround. For this I would like to refactor this class to make it
> modular. Therefore I need refactoring support, thus the automated testing.
> Very simple.
>

That's totally okay by me. I'm not trying to suggest that you do it
differently.

Work on the things that interest you. That's how we've gotten this far. :-)

>> How about this:
>>
>> Save files: {game name}-{current round}-{unique identifier}.save
>> Index file: savegame-tests.txt
>> The save game will simply be something like 1870-OR5-1.save,
>> 1830-SR2-4.save, etc.
>
>
> The extension for the save files can be changed in test.properties, but I
> prefer the .rails extension to the .save (which could be anything).
>
> I would suggest then to reverse the order and add a game end indicator:
>
> {game name}_{unique identifier}_{current round}{[_e,_b]}
>
>  where e indicates normal end (bank out of money), b indicates bankrupt.
>
> Examples: 1830_4_sr2.rails, 1830_4_or7_e.rails, 1830_3_or5_b.rails
>


That's fine. I'm less concerned about the extension as I am about
trying to fit too much information into the file name.


>>
>> Then, the index file will track our notes about each saved game, and
>> what tests it applies to.  I think there's going to need to be more
>> information than can be contained within the filename itself.
>
>>
>> Alternately, we can have each saved game with it's own
>> {conventiion}.txt file that documents this, but that may be overkill.
>>
>> Of course, all of this should probably go under /test/savedgames, or
>> something similarly named and organized away from the rest of the
>> code.
>>
>
> That is how it is laid out: Root save directory can be changed in
> test.properties. Currently set to test/data
>

Perfect.  Sounds like you're on the right track. :-)

---Brett.