Recurring fails on windows tests and the recent CentOS failure

[roger peppe]

On 15 July 2015 at 17:03, Nate Finch <nate.finch at canonical.com> wrote:
> I think the answer is - if there are methods on a suite that don't require
> setup/teardown, maybe they should be moved to standalone functions or
> methods on non-suite types, so that we can know what requires setup/teardown
> and what doesn't.   That way we can use those functions without incurring
> the cost of running all the setups and teardowns.  Then we can do Roger's
> suggestion of ensuring that we don't forget to hook up setup and/or
> teardown.

All fixture types ("Suite" is a misnomer that should have been fixed
ages ago) should define all setup and teardown methods (even if empty),
and any type that embeds a fixture type should call them.

Otherwise if a fixture is changed to add a new setup/teardown method,
any code which uses it is now broken because no type which embeds
it will be calling the new method.

There was a point a couple of years ago when I went through
changing all the juju-core Suite types to have all setup/teardown
methods, spurred by a series of bugs caused by precisely that problem.

  cheers,
    rog.

> On Wed, Jul 15, 2015 at 3:31 PM William Reade <william.reade at canonical.com>
> wrote:
>>
>> On Wed, Jul 15, 2015 at 7:19 PM, Nate Finch <nate.finch at canonical.com>
>> wrote:
>>>
>>> Or maybe we should just code teardown so that if setup isn't  called, we
>>> don't do teardown either.  Lots of times I see people say "just include
>>> basesuite for X method" and since I don't need whatever its setup does, I
>>> don't call it in my setup  I don't think that's a programmer error, per se.
>>
>>
>> That's cavalier at best, isn't it? If I see an embedded test suite I
>> expect to be able to use its full capabilities.
>>
>> It's upsetting to discover that your test modifications don't work right
>> because someone didn't set the base suite up; to discover that they didn't
>> set the base suite up *on purpose* adds significant insult to the injury. If
>> you don't need it, don't embed it; if you do need it, please use it
>> properly.
>>
>> Cheers
>> William
>>
>>>
>>>
>>> On Wed, Jul 15, 2015 at 7:39 AM roger peppe <rogpeppe at gmail.com> wrote:
>>>>
>>>> It shouldn't be hard to write some code (using go/types) that
>>>> automatically checks
>>>> for these invariants.
>>>>
>>>>
>>>> On 15 July 2015 at 07:05, John Meinel <john at arbash-meinel.com> wrote:
>>>> >
>>>> >
>>>> > On Wed, Jul 15, 2015 at 2:44 AM, Bogdan Teleaga
>>>> > <bteleaga at cloudbasesolutions.com> wrote:
>>>> >>
>>>> >> Hello everybody,
>>>> >>
>>>> >> Lately I've been noticing a couple of failures regarding a new
>>>> >> testing
>>>> >> feature introduced on windows.
>>>> >>
>>>> >> Without going into implementation details too deep the main idea is
>>>> >> that
>>>> >> whenever a new suite is created
>>>> >> that inherits from a base suite and a SetUpX function is defined, it
>>>> >> needs
>>>> >> to call the SetUpX function of the
>>>> >> base suite. The main reason would be that if that's not done we will
>>>> >> not
>>>> >> do set up for any test, but we will do
>>>> >> a tear down. To some extent it might be worth it to add some
>>>> >> annotations
>>>> >> to the failure message, since it might
>>>> >> come up more often and it is not immideately obvious why.
>>>> >
>>>> >
>>>> > That's certainly an expectation of any sort of function that a type
>>>> > overloads from an embedded type. If people are overriding SetUp and
>>>> > not
>>>> > calling the embedded SetUp that's going to cause all sorts of bugs (we
>>>> > do
>>>> > test suite isolation, logging changes, HOME directory setup in base
>>>> > types).
>>>> > I'm actually surprised that things worked if people are doing so. I
>>>> > guess
>>>> > some things like Isolation are things you don't notice until late,
>>>> > because
>>>> > you end up with files littered where they shouldn't be.
>>>> >
>>>> > John
>>>> > =:->
>>>> >
>>>> >>
>>>> >>
>>>> >> The bugs caused by this so far are:
>>>> >> - https://bugs.launchpad.net/juju-core/+bug/1474382
>>>> >> - https://bugs.launchpad.net/juju-core/+bug/1471941
>>>> >>
>>>> >> Another issue is that a recent bugfix stopped CentOS completely from
>>>> >> working.
>>>> >> The series could not be detected anymore because the map was changed
>>>> >> *and*
>>>> >> the test that
>>>> >> was using actual data from /etc/os-release was modified to mirror
>>>> >> this
>>>> >> change. Until we get the CI
>>>> >> for CentOS up and running, but even as a general thing for that
>>>> >> matter,
>>>> >> please consider the old
>>>> >> content of the tests and their intention before modyfing them to fit
>>>> >> the
>>>> >> changes in the code.
>>>> >>
>>>> >> For more details on the CentOS bug:
>>>> >> https://bugs.launchpad.net/juju-core/+bug/1470150
>>>> >>
>>>> >> And that would be all for now, I'll let you get back to the other
>>>> >> hundred
>>>> >> emails.
>>>> >>
>>>> >> Thanks,
>>>> >> Bogdan
>>>> >>
>>>> >> --
>>>> >> Juju-dev mailing list
>>>> >> Juju-dev at lists.ubuntu.com
>>>> >> Modify settings or unsubscribe at:
>>>> >> https://lists.ubuntu.com/mailman/listinfo/juju-dev
>>>> >>
>>>> >
>>>> >
>>>> > --
>>>> > Juju-dev mailing list
>>>> > Juju-dev at lists.ubuntu.com
>>>> > Modify settings or unsubscribe at:
>>>> > https://lists.ubuntu.com/mailman/listinfo/juju-dev
>>>> >
>>>>
>>>> --
>>>> Juju-dev mailing list
>>>> Juju-dev at lists.ubuntu.com
>>>> Modify settings or unsubscribe at:
>>>> https://lists.ubuntu.com/mailman/listinfo/juju-dev
>>>
>>>
>>> --
>>> Juju-dev mailing list
>>> Juju-dev at lists.ubuntu.com
>>> Modify settings or unsubscribe at:
>>> https://lists.ubuntu.com/mailman/listinfo/juju-dev
>>>
>