Re: Uniter tests for update-status hook - BLOCKER
On 20/07/2015, Tim Penhey tim.pen...@canonical.com wrote: Earlier today I was investigating this CRITICAL BLOCKER bug: https://bugs.launchpad.net/juju-core/+bug/1475724 At first I thought that bug was referring to a different one, which I fixed by skipping a part of a test that was using chmod to make unit enter an error state. I filed a bug for the fixing of the skip: https://bugs.launchpad.net/juju-core/+bug/1476060 However on deeper looking into the first bug, it seems that it is all timing related. Unfortunately tracking failures in the uniter tests is complicated. I've seen four named tests fail much more often in recent days: TestUniterRelations http://reports.vapour.ws/releases/issue/559b486d749a566c38b6a7bc TestUniterCollectMetrics http://reports.vapour.ws/releases/issue/555b70eb749a563f74eb83ec TestUniterSendMetrics http://reports.vapour.ws/releases/issue/55acde69749a560b02ac8d06 TestUniterUpdateStatusHook http://reports.vapour.ws/releases/issue/55ace932749a56072b3d3637 However it's hard to manage as we have issues in the suite in general, and uniter test output is not easy to deal with. The logs are giant, the actual failure lines tend to be non-informative with the real cause several screens up in the log, multiple tests have basically the same problems with common code... So, I'm not sure what bugs we want to file to track the work to get master in a good state. As best as I can work out we have: 1) A regression on windows from the 16th, probably from this metrics landing: http://reviews.vapour.ws/r/2173/ 2) An earlier regression on all platforms tracked in this bug: https://bugs.launchpad.net/juju-core/+bug/1475724 3) A more general problem with the TestUniterSendMetrics. I'm not even completely sure which of these your windows test skip resolves, but I assume the first? Fun, huh. [1] We should seriously start thinking how to gate landings on the unit tests passing on amd64, ppc, and windwos. I'd love to gate on the windows tests, but don't want to get yelled at. Recently, three test runs at 40mins each has not been enough to get a passing suite reliably, but maybe with this latest batch of fixes that becomes more reasonable again. Martin -- Juju-dev mailing list Juju-dev@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/juju-dev
Re: Uniter tests for update-status hook - BLOCKER
On 20/07/15 07:57, William Reade wrote: On Mon, Jul 20, 2015 at 6:42 AM, Tim Penhey tim.pen...@canonical.com mailto:tim.pen...@canonical.com wrote: Hi folks, Earlier today I was investigating this CRITICAL BLOCKER bug: https://bugs.launchpad.net/juju-core/+bug/1475724 I'll talk about the specific bug to begin with, but there's a much more important bit further down. If you're pressed for time, skip down to the point marked THE IMPORTANT BIT. But as you can see above, there was not a second update-status before the config-changed (but there is one after on both). Really the test doesn't care one hoot about the update-status hook, all it really cared about checking was the config-changed. [2] The immediate bit: So, I agree we have a big elephant on the room and we might have all been looking the other way (most likely distracted by the pack of velocirraptors on the other side). We ought to sit and talk about this tech-debt negotiation, as any debt negotiation this takes a couple of rounds of discussion around a table, some shouting and a conclusion that might let none of us completely happy but will produce the best outcome in terms of reliability and functionality for the experience/service we are trying to provide here. In the immediate however, this is blocking master so Ill be working on a solution that solves the symptom at hand as we know this works in practice it is a matter of better defining the tests for now while we work ASAP in a better foundation for at least idle and whatever is built on top of it. As a side note, we need to spread some education about the uniter, its innards and the design philosophy around it since much of the added code on it seems a result of just throwing a case into the select, see what it did and then accommodate to it (my very first approach included). Cheers. -- Horacio -- Juju-dev mailing list Juju-dev@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/juju-dev
Re: Uniter tests for update-status hook - BLOCKER
On 20/07/2015, Martin Packman martin.pack...@canonical.com wrote: So, I'm not sure what bugs we want to file to track the work to get master in a good state. As best as I can work out we have: Okay, becoming clearer now, 1) Windows regression from uniter-status change: http://reviews.vapour.ws/r/1979/ Fixed by Tim adding the skip, tracked by this bug: https://bugs.launchpad.net/juju-core/+bug/1476060 2) An earlier regression on all platforms tracked in this bug: https://bugs.launchpad.net/juju-core/+bug/1475724 3) Metrics breakage that should now be fixed by this change, not tracked in a bug: http://reviews.vapour.ws/r/2143/ 4) Different metrics breakage fixed as part of this bug: https://bugs.launchpad.net/juju-core/+bug/1475271 Plus unreliable tests, plus feature branches that aren't all up to date with trunk. Martin -- Juju-dev mailing list Juju-dev@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/juju-dev
Re: Uniter tests for update-status hook - BLOCKER
On Mon, Jul 20, 2015 at 12:42 AM Tim Penhey tim.pen...@canonical.com wrote: Aside from all this work, it is becoming REALLY IMPORTANT that we stop writing terrible, wasteful, full integration type tests when what we really care about testing is some aspect of uniter internals. I know that it is just simpler to copy what is there and make more, but it is better to write smaller, targeted tests that just test what you are wanting to assert. +100 If you start writing tests and you reach for JujuConnSuite, stop. You should be able to get to 80% code coverage (and near 100% logic coverage) without using anything outside your package. Just provide some stub implementations of interfaces and assert that the logic run against those interfaces is correct. If you have too many dependencies on code outside your package, start writing interfaces and refactor your code to use them instead of explicitly relying on external types. -- Juju-dev mailing list Juju-dev@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/juju-dev
Re: Uniter tests for update-status hook - BLOCKER
On Mon, Jul 20, 2015 at 10:57 AM roger peppe rogpe...@gmail.com wrote: On 20 July 2015 at 14:11, Martin Packman martin.pack...@canonical.com wrote: The logs are giant, the actual failure lines tend to be non-informative with the real cause several screens up in the log, multiple tests have basically the same problems with common code... FWIW I often delete all lines containing the string [LOG] before looking at the output - it helps me to see the wood from the trees. Also FWIW, this is why I wrote https://github.com/natefinch/nolog - it filters out the spammy logging by default, and offers a few other niceties (many that Horacio added). It's a go application, so just 'go get' it. I know that doesn't help when reading CI output, but it is super handy when reproducing a problem locally. (Yes you could run some combination of linux CLI invocations to do the same thing - this is easier). -- Juju-dev mailing list Juju-dev@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/juju-dev
Re: Uniter tests for update-status hook - BLOCKER
On Mon, Jul 20, 2015 at 4:57 PM, roger peppe rogpe...@gmail.com wrote: That's somewhat harder with the uniter, because its very state-dependent channel operations make it awkward to write a uniform outer select loop. If I were to do it, off the top of my head, I might consider making uniter.Mode (which BTW should not really be exported) into an interface and each of the existing mode functions into a separate types. Yeah; oddly enough, it was partly my imagining going in more-or-less that direction (but implementing the modes in another package) that led to them being written as exported. However, I think the mode approach has outlived its usefulness. After a couple of years, Modes are now either trivial or bloated; in ModeAbide in particular, there are too many possible inputs and reactions to keep track of them in one select loop. And it's becoming increasingly inconvenient to depend on the golang scheduler; and, basically, we need an operation queue that self-prioritises/compacts as events come in. It won't be trivial -- in particular, we need to disentangle the local-state storage from the operations that currently generate it -- but the queue can be synchronous; it can be composed of smaller queues that manage priority of closely-related hooks (as relation hooks are implemented today); and it can actually be unit-tested in adequate detail. And the event delivery might still not be beautiful, but I'm pretty sure we can do better than the existing Filter if we don't constrain ourselves by optimizing the interface for delivering events to mode loops. Cheers William -- Juju-dev mailing list Juju-dev@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/juju-dev
Re: Uniter tests for update-status hook - BLOCKER
On Mon, Jul 20, 2015 at 3:05 PM roger peppe roger.pe...@canonical.com wrote: On 20 July 2015 at 19:41, Nate Finch nate.fi...@canonical.com wrote: You should be able to get to 80% code coverage (and near 100% logic coverage) without using anything outside your package. Shouldn't those numbers be the other way around? I don't see how you could possibly get to ~100% of logic coverage if only 80% of the code is covered. Yes, you're right, any runnable code is logic. But there's often some uninteresting glue code that is not really the meat of the logic, that may be hard to test and unlikely to break. I guess that's really too vague as a guideline, though. My point is - you should be able to test most of your code without having to spin up a server - even a fake test server. -- Juju-dev mailing list Juju-dev@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/juju-dev
