Response to your last code review and FLUID-3116, FLUID-3117

[michelle.dsouza at utoronto.ca]

Hi Laurel,

Good job on refactoring these tests. Please see my answer below.


Quoting "Laurel A. Williams" <laurel.williams at utoronto.ca>:


> As I was working with the tests, I noted that we reproduced the exact
> same data in the tests. I refactored to use only one copy of the data,
> but perhaps it was a case of "premature optimization".
>
> Note though, since this is a piece of demo code, the
> fluid.customBuild.demo.completeFluidInfusionData data does not need to
> change - it could stay the same even as infusion expands, since the
> data for the 'live customBuilder' will always come from the
> build.properties file and not from this data structure. However, I'm
> thinking that people will be tempted to change the data as the real
> infusion expands, in which case they will have to change the tests to
> reflect those changes...and some of the hard coding will be a pain to
> change.
>
> I did try to address this somewhat by providing some constants for the
> indexes of the specific modules in the data which you wrote tests
> around. In customBuild-tests.js you can find:
>
>        var INLINE_EDIT_INDEX = 7;
>        var PAGER_INDEX = 8;
>        var PROGRESS_INDEX = 9;
>        var REORDERER_INDEX = 10;
>        var UIOPTIONS_INDEX = 12;
>
> So, I think that outlines all of the details around this data
> structure...what do you think...should it simply be duplicated in both
> the tests and the demo to prevent people from breaking the tests
> inadvertently? I'm not convinced, but honestly don't think it is
> critical one way or the other.
>

I think it would be best to have a single copy of the demo data and  
have the tests run against that. It's true that if someone updates the  
demo data the tests may break, however, that might be a good thing. If  
we at some point change the structure of the data, not just the  
content, we would want the tests to break. It is more likely that  
someone will update the demo data rather then the test data in this  
case.

Also, as you said, I don't feel there is any need to update the demo  
data when new modules are added. Unless of course the pattern of the  
additional module is different - a special case. In that case, we'd  
want the test data updated anyway because we would need to write a new  
test.

As someone coming into these tests cold, I actually like the named  
indexes because it makes it clear to me in the test what is being  
tested. It's easier for me to look in the data for 'pager' rather then  
index 8.

Hope this helps,

Michelle