Re: Defaulting tests internal to the package

2016-01-24 Thread Rick Harding
I've got to toss another +1 for tests at both levels. One set of tests are
tests against your contract to outsiders. Another is confidence that your
internals are resilient. There are a ton of cases I can think of such as
internal code that validates changes in state, validates various forms of
input, deals with internal changes to document structure over time.
Ideally, when an external contract test fails, one of the internal ones
just blew up to point directly at the culprit within all your internal
code.

Yes, it can mean that internal tests need to be kept up to date more as the
internals change, but even then tests provide another layer of "did you
cover all these cases in your refactoring".

On Sun, Jan 24, 2016 at 4:47 PM Tim Penhey  wrote:

> I'm going to throw my cool-blue paint against the bike shed.
>
> I disagree that we should change the default expectations of package tests.
>
> By default, package tests should be external tests, small and fast, not
> full stack, and parameterize from above rather than patch.
>
> However, I also see much value in internal unit tests for exactly the
> same reason as Nate and Roger. It gives me confidence in my
> implementation. These tests have nothing to do really with the exposed
> interface of the package, but with the current implementation.
>
> This whole issue is close to my heart right now as I deal with model
> migrations. Many of the tests I am writing are internal tests.
>
> Tim
>
> --
> Juju-dev mailing list
> Juju-dev@lists.ubuntu.com
> Modify settings or unsubscribe at:
> https://lists.ubuntu.com/mailman/listinfo/juju-dev
>
-- 
Juju-dev mailing list
Juju-dev@lists.ubuntu.com
Modify settings or unsubscribe at: 
https://lists.ubuntu.com/mailman/listinfo/juju-dev


Re: Defaulting tests internal to the package

2016-01-24 Thread Nate Finch
I think the idea that tests of an internal function are worthless because
you might just not be using that function is a fairly specious argument.
That could be applied to unit tests at any level.  What if you're not using
the whole package?  By that logic, the only reliable tests are full stack
tests.

I think that William's position is correct in an ideal world.  The public
interface is the only contract you really need to worry about.  In theory,
all your packages would be simple enough that this would not be difficult,
and the tests would be straightforward.  However, in practice, some
packages are fairly complex beasts, and it can literally be an order of
magnitude easier to test a bit of logic internally than to test it
externally.  And I don't just mean easier in terms of time it takes to
write (although that shouldn't be ignored - we do have deadlines), but also
in simplicity of the code in the tests... the more complicated the code in
the tests, the harder they are to get right, the more difficult it is for
other people to understand them, and the more difficult it is to maintain
them.

Even if you have perfect code coverage, using inversion of control and
dependency injection to be able to do every test externally, you can still
have bugs that slip through, either due to weaknesses in your tests, or
even just poorly coded mocks that don't actually behave like the production
things they're mocking.  Isolating a piece of code down to its bare bones
can make its intended behavior a lot easier to define and understand, and
therefore easier to test.

I think small scope internal unit tests deliver incredible bang for the
buck.  They're generally super easy to write, super easy to understand, and
give you confidence that you haven't screwed up a piece of logic. They're
certainly not ironclad proof that your application as a whole doesn't have
bugs in it... but they are often very strong proof that this bit of logic
does not have bugs.

While I have seen plenty of bugs that were caused by a developer not using
a piece of code he/she was supposed to use, it has almost always been
exported code that wasn't used - a utility function in another package, for
example.  Certainly it's possible you could accidentally stop using an
internal function and no have anyone notice... but I think that's fairly
unlikely compared to the concrete benefits you get from simpler,
faster-to-write internal tests.

-Nate
-- 
Juju-dev mailing list
Juju-dev@lists.ubuntu.com
Modify settings or unsubscribe at: 
https://lists.ubuntu.com/mailman/listinfo/juju-dev


Re: Tags and object IDs

2016-01-24 Thread John Meinel
On Sat, Jan 23, 2016 at 1:28 AM, William Reade 
wrote:

> On Fri, Jan 22, 2016 at 9:53 PM, Nate Finch 
> wrote:
>
>> Working in the model layer on the server between the API and the DB.
>> Specifically in my instance, an API call comes in from a unit, requesting
>> the bytes for a resource.  We want to record that this unit is now using
>> the bytes from that specific revision of the resource.  I have a pointer to
>> a state.Unit, and a function that takes a Resource metadata object and some
>> reference to the unit, and does the actual transaction to the DB to store
>> the unit's ID and the resource information.
>>
>
> I'm a bit surprised that we'd be transferring those bytes over an API call
> in the first place (is json-over-websocket really a great way to send
> potential gigabytes? shouldn't we be getting URL+SHA256 from the apiserver
> as we do for charms, and downloading separately? and do we really want to
> enforce charmstore == apiserver?); and I'd point out that merely having
> agreed to deliver some bytes to a client is no indication that the client
> will actually be using those bytes for anything; but we should probably
> chat about those elsewhere, I'm evidently missing some context.
>

So I would have expected that we'd rather use a similar raw
HTTP-to-get-content instead of a JSON request (given the intent of
resources is that they may be GB in size), but regardless it is the intent
that you download the bytes from the charm rather from the store directly.
Similar to how we currently fetch the charm archive content itself.
As for "will you be using it", the specific request from the charm is when
it calls "resource-get" which is very specifically the time when the charm
wants to go do something with those bytes.

John
=:->


> But whenever we do record the unit-X-uses-resource-Y info I assume we'll
> have much the same stuff available in the apiserver, in which case I think
> you just want to pass the *Unit back into state; without it, you just need
> to read the doc from the DB all over again to make appropriate
> liveness/existence checks [0], and why bother unless you've already hit an
> assertion failure in your first txn attempt?
>
> Cheers
> William
>
> [0] I imagine you're not just dumping (unit, resource) pairs into the DB
> without checking that they're sane? that's really not safe
>
>
>> On Fri, Jan 22, 2016 at 3:34 PM William Reade <
>> william.re...@canonical.com> wrote:
>>
>>> Need a bit more context here. What layer are you working in?
>>>
>>> In general terms, entity references in the API *must* use tags; entity
>>> references that leak out to users *must not* use tags; otherwise it's a
>>> matter of judgment and convenience. In state code, it's annoying to use
>>> tags because we've already got the globalKey convention; in worker code
>>> it's often justifiable if not exactly awesome. See
>>> https://github.com/juju/juju/wiki/Managing-complexity#workers
>>>
>>> Cheers
>>> William
>>>
>>> On Fri, Jan 22, 2016 at 6:02 PM, Nate Finch 
>>> wrote:
>>>
 I have a function that is recording which unit is using a specific
 resource.  I wrote the function to take a UnitTag, because that's the
 closest thing we have to an ID type. However, I and others seem to remember
 hearing that Tags are really only supposed to be used for the API. That
 leaves me with a problem - what can I pass to this function to indicate
 which unit I'm talking about?  I'd be fine passing a pointer to the unit
 object itself, but we're trying to avoid direct dependencies on state.
 People have suggested just passing a string (presumably
 unit.Tag().String()), but then my API is too lenient - it appears to say
 "give me any string you want for an id", but what it really means is "give
 me a serialized UnitTag".

 I think most places in the code just use a string for an ID, but this
 opens up the code to abuses and developer errors.

 Can someone explain why tags should only be used in the API? It seems
 like the perfect type to pass around to indicate the ID of a specific
 object.

 -Nate

 --
 Juju-dev mailing list
 Juju-dev@lists.ubuntu.com
 Modify settings or unsubscribe at:
 https://lists.ubuntu.com/mailman/listinfo/juju-dev


>
> --
> Juju-dev mailing list
> Juju-dev@lists.ubuntu.com
> Modify settings or unsubscribe at:
> https://lists.ubuntu.com/mailman/listinfo/juju-dev
>
>
-- 
Juju-dev mailing list
Juju-dev@lists.ubuntu.com
Modify settings or unsubscribe at: 
https://lists.ubuntu.com/mailman/listinfo/juju-dev