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.
IIUC it's best practice for a unit test that all context be within the test. 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. 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 > >
