On 20/06/2019 16:07, Eric Barnhill wrote:
Sorry for the slow reply, I thought I sent this yesterday.
I agree from a code architecture standpoint such a refactoring makes sense.
However from the perspective of unit tests it makes it no longer a unit
test.
It is still testing a unit. Just the test code is spread out over
multiple files so it can be reused.
IIUC it's best practice for a unit test that all context be within the
test.
If the purpose of the test is to ensure two pieces of code do the same
thing then perhaps a rewrite as a parameterized test. Unfortunately
BigFraction and Fraction do not share a common interface that the test
is checking. So a parameterized test is not as simple as passing in an
instance of one or the other (or a reference to the factory constructor
method). Putting all the test cases in a single place rather than
duplicating them in two files seems like the most maintainable going
forward.
If additional context is required it fails to meet the definition of
a unit test and is instead an integration test, and the function being
tested may require rethinking.
Depends what you define as a unit test. I'd say the unit was BigFraction
or Fraction. An integration test is something that must be tested with
coherant components working together to provide functionality. You are
not doing that.
I'd say that if you executed the all the tests in the Test file and it
achieves 100% coverage of the class it is aimed at then it is a unit
test. Even if your Test file uses code from other places.
In this instance the code from other places amounts to a list of
arguments and expected results. Duplicating this data just to make the
Test file standalone is less maintainable.
This results in unit tests often being clunkily and awkwardly coded, but I
think it is the way they are typically written and it has its reasons.
So I am +0 .
On Thu, Jun 20, 2019, 02:01 Heinrich Bohne <heinrich.bo...@gmx.at> wrote:
A quick looks shows that the BigFractionTest does have test cases for
very large numbers. However the add, subtract, divide and multiply tests
and a few others just use values that would work with Fraction. Possibly
these can be moved to a shared common tests location too.
That's what I was thinking too – the draft was by no means intended to be
complete, I just created it to give a general idea of how I would go about
implementing this. I'll work some more on it before I create an actual pull
request.
On 6/20/19 10:40 AM, Alex Herbert wrote:
On 20 Jun 2019, at 00:54, Heinrich Bohne <heinrich.bo...@gmx.at> wrote:
An awful lot of code is duplicated between FractionTest and
BigFractionTest. Often, the test cases in the two classes only differ in
the types they use (e.g. Fraction vs. BigFraction), but the actual
values the tests use are the same.
I think this could be mitigated by adding a new class that stores the
values for these common test cases, and the classes FractionTest and
BigFractionTest retrieve the values from this class and only implement
the test patterns.
I created a draft here:
https://github.com/Schamschi/commons-numbers/commit/53906afd991cd190f1a05beb0952a40ae6c6ea3f
Any opinions on this?
1. BigFraction should work the same way as Fraction when the numbers are
the same
So collecting the common tests together makes sense. The change in the
PR looks good.
2. BigFraction should work with numbers that cannot be handled by
Fraction
A quick looks shows that the BigFractionTest does have test cases for
very large numbers. However the add, subtract, divide and multiply tests
and a few others just use values that would work with Fraction. Possibly
these can be moved to a shared common tests location too.
Then variants added using BigInteger arguments just to make sure the Big
part of BigFraction is working.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
For additional commands, e-mail: dev-h...@commons.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
For additional commands, e-mail: dev-h...@commons.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
For additional commands, e-mail: dev-h...@commons.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
For additional commands, e-mail: dev-h...@commons.apache.org
