> On June 21, 2012, 2:08 p.m., Matěj Laitl wrote:
> > tests/core/collections/TestCollection.cpp, line 203
> > <http://git.reviewboard.kde.org/r/105166/diff/2/?file=70025#file70025line203>
> >
> >     While entire code shouldn't be here, I wonder why you just dont
> >     QCOMPARE( collection->usedCapacity(), 0.0 )? You really want to pretend 
> > static_cast doesn't exist and use it only in at a least resort.
> 
> Jasneet Bhatti wrote:
>     0.0 is a double and usedCapacity returns a float, so there was a warning 
> on compilation. Used the cast to eliminated the warning.
>     
>     http://qt-project.org/doc/qt-4.8/qtest.html#QCOMPARE
>     "In the case of comparing floats and doubles, qFuzzyCompare() is used for 
> comparing. This means that comparing to 0 will likely fail. One solution to 
> this is to compare to 1, and add 1 to the produced output."
>     
>     I'm not sure if the above applies for 0.0 too, so added 1 to not take a 
> chance.
> 
> Matěj Laitl wrote:
>     > 0.0 is a double and usedCapacity returns a float, so there was a 
> warning on compilation. Used the cast to eliminated the warning.
>     
>     Oh I see. Then preferred way to cast basic types is to use their 
> "constructor": float( 0.0 )
>     
>     > "In the case of comparing floats and doubles, qFuzzyCompare() is used 
> for comparing. This means that comparing to 0 will likely fail. One solution 
> to this is to compare to 1, and add 1 to the produced output."
>     
>     I see, then you can use QVERIFY( actual == expected ) to avoid 
> qFuzzyCompare().
>     
>     > I'm not sure if the above applies for 0.0 too, so added 1 to not take a 
> chance.
>     
>     For doubles and floats, 0.0 == 0.

Ok. Will do this.


> On June 21, 2012, 2:08 p.m., Matěj Laitl wrote:
> > tests/core/collections/TestCollection.cpp, line 34
> > <http://git.reviewboard.kde.org/r/105166/diff/2/?file=70025#file70025line34>
> >
> >     Strange indentation again. Hmm?
> 
> Jasneet Bhatti wrote:
>     Don't understand this one.
>     There are two levels of indentation and the variable definition is 
> according to intro_and_style.
>     Please explain what is the fault.
> 
> Matěj Laitl wrote:
>     There is no indentation from what I see here.

Well, the patch I uploaded and my repo both show the indentation, not sure why 
it isn't visible to you ?


- Jasneet


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/105166/#review14948
-----------------------------------------------------------


On June 21, 2012, 1:38 p.m., Jasneet Bhatti wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/105166/
> -----------------------------------------------------------
> 
> (Updated June 21, 2012, 1:38 p.m.)
> 
> 
> Review request for Amarok and Sven Krohlas.
> 
> 
> Description
> -------
> 
> This patch implements a unit test for core/collections/Collection
> 
> There are abstract classes to be tested as well, which can only be done when 
> subclasses define the pure virtual functions. So tests for those will be done 
> along with the subclasses.
> 
> 
> Diffs
> -----
> 
>   tests/core/collections/CMakeLists.txt 2efd1fe 
>   tests/core/collections/TestCollection.h PRE-CREATION 
>   tests/core/collections/TestCollection.cpp PRE-CREATION 
> 
> Diff: http://git.reviewboard.kde.org/r/105166/diff/
> 
> 
> Testing
> -------
> 
> Test passes on my repository
> 
> 
> Thanks,
> 
> Jasneet Bhatti
> 
>

_______________________________________________
Amarok-devel mailing list
Amarok-devel@kde.org
https://mail.kde.org/mailman/listinfo/amarok-devel

Reply via email to