=== thumper-gym is now known as thumper [00:19] * thumper is back to go through the review [01:09] davecheney: https://codereview.appspot.com/8348043 has been updated following fwereade__'s review [01:09] more looks would be good though [01:18] * davecheney looks [01:23] * thumper needs food, goes hunting in the kitchen [01:24] * bigjools imagines thumper spearing a bear roaming in his kitchen [01:25] http://creativeroots.org/2011/08/kiwi-anatomy-t-shirt/ [01:27] haha [01:54] davecheney: how many people do we have in this TZ that can approve my branch? [01:54] * thumper wonders when jam starts [01:56] bigjools: not a bear, but I did get some salami and cheese :) [01:56] lucky me, they didn't put up much resistence [01:57] thumper: I found a hot cross bun that was toasted [01:57] bigjools: did you have to light the fire to toast it? [01:57] or was it left by a passer-by [01:57] ? [01:57] I could use Ian's dog's breath [01:58] it's enough to curl hair [01:58] heh [01:58] * thumper needs to collect a young child from school [01:59] bbs [04:27] davecheney: ping [04:30] davecheney: https://code.launchpad.net/~thumper/juju-core/package/+merge/157020 [04:31] * thumper off until the meeting [06:01] jtv1: just trying to sort out what branch you want reviewed === ChanServ changed the topic of #juju-dev to: https://juju.ubuntu.com | On-call reviewer: dimitern | Bugs: 2 Critical, 53 High - https://bugs.launchpad.net/juju-core/ [06:13] morning all [06:15] hi dimitern [06:16] Hi jam, hi dimitern [06:16] jam: I was about to bring it up... Had some trouble with lbox, but had to go with the normal way instead: [06:17] https://code.launchpad.net/~maas-maintainers/juju-core/maas-provider-skeleton/+merge/157025 [06:17] I'm just fixing the conflicts. [06:29] Oh dear. I'm getting build errors, but I get the same ones in trunk. [06:30] Nonexistent functions in "log," and more. [06:31] jtv1: which revision are you on? [06:31] 1104 [06:33] jtv1: me too, just pulled trunk and running the tests; can you paste the errors? [06:34] Coming up. [06:34] I get the exact same ones for trunk and our branch, so I guess that's a good sign w.r.t. ours. [06:35] dimitern: http://paste.ubuntu.com/5675740/ [06:35] all passes here [06:36] jtv1: these are build errors, not test failures - something is not updated I think [06:38] I'll run a "go get -u juju-core" then I guess [06:38] jtv1: well, it'll possibly screw up your working dir [06:39] jtv1: if you have branches, etc. - maybe try changing $GOPATH to a new dir and get it afresh? [06:39] Ouch. [06:39] OK, I'll try that, thanks [06:43] morning all! [06:44] Hi rogpeppe [06:44] jtv1: hiya [06:44] rogpeppe: hey [06:44] * davecheney waves [06:44] davecheney: hi :) [06:45] davecheney: good evening; dimitern: yo! [06:47] fwereade__: I have a branch for you https://codereview.appspot.com/8330043/ [06:50] dimitern, hey, just reviewed [06:52] fwereade__: cheers! btw the pending/starting is just the first step, the MA will set the correct status in a follow-up I think [06:52] dimitern, I think I would prefer to set nothing than to set something that's only mostly correct [06:53] dimitern, there's an argument to be made that "provisoned" or "starting" or whatever can be inferred perfectly well from InstanceId [06:53] fwereade__: ok, i'll think about it some more [06:53] dimitern, cheers [06:56] fwereade__: why do you think breakJuju should panic in the provisioner test? [07:01] is the meeting now or in 1h ? [07:02] mramm2: ping [07:03] * davecheney wonders if he has time to order a pizza before the meeting [07:03] so it's not now [07:04] nope, still 6pm in AU [07:04] meeting is at 7pm [07:05] so when does AU switch to DST? [07:05] soon [07:06] dimitern: 7th Apr [07:06] I see [07:06] but not Queensland [07:06] they never adopted daylight savings because they were worried it would confuse the Cows [07:06] so you're last to go - we already switched last sunday, and US - before 3 weeks [07:07] :D cool [07:07] dimitern: even inside australia the 7 states shift at different times [07:07] /s/times/dates [07:07] because when something is hard, it helps to make it even harder by not coordinating [07:07] * davecheney says fuckit, and orders a pizza' [07:09] interesting :) and opinionated [07:26] dimitern, sorry [07:26] dimitern, the provisioner test should not have a JujuHome [07:27] fwereade__: it's not using it really [07:27] dimitern, it runs in the cloud and doesn't have an environments.yaml or any of the associated crap [07:27] dimitern, yes it is [07:27] fwereade__: it has environ config though [07:27] dimitern, it's getting the environment from environments.yaml [07:28] dimitern, which invokes a whole bunch of crap that is *wrong* for this context [07:28] dimitern, I see why it works though [07:28] fwereade__: well, it's changing that, not reading it, but I see your point [07:28] dimitern, it's because JujuConnSuite is broken [07:28] dimitern, but fixing that is out of scope [07:28] dimitern, it's changing it and then reading it, right? [07:29] fwereade__: I'll split the "make broken env.yaml" from "make broken environ config" for a given dummy method [07:29] fwereade__: not [07:29] dimitern, then why's it writing it? :) [07:29] fwereade__: reading it - just changing it and returning config.Config [07:29] :) [07:30] fwereade__: because breakJuju seemed like a good candidate for that, already used in cmd tests [07:30] fwereade__: i changed it to reuse it, but it diverged too much from the original and now it's doing 2 unrelated things [07:30] dimitern, sure, but the dummy provider already has exactly the capability you need -- you just set one field in the existing config [07:31] dimitern, I think that just doing that in the provisioner test is neatest [07:31] fwereade__: wasn't sure, but now I see it better (after having done it the wrong way first) :) [07:31] dimitern, don't worry, this stuff is nasty to begin with and only made worse by the essentially-random contexts in which we run tests [07:32] fwereade__: how about adding an exported call to the dummy provider to make breaking methods easier? [07:32] dimitern, honestly its not that much work to change a config [07:33] dimitern, cfg, err := environ.Config().Apply(map[string]interface{}{"broken": "StartInstance"}); c.Assert(err, IsNil); err = environs.SetConfig(cfg); c.Assert(err, IsNil) [07:33] fwereade__: it's not obvious and it's definitely useful to have a shortcut I think, for future use as well [07:34] fwereade__: (and documented properly, rather than having to read through the source to figure out how) [07:34] dimitern, isn't that a method that'll be used in exactly one place? [07:35] fwereade__: in any place that needs to break the dummy provider's methods without needing environments.yaml [07:35] fwereade__: i'm sure there'll be others [07:36] dimitern, I suspect that `UpdateConfig(env Environ, attrs map[string]interface{}) error` would actually be useful [07:36] fwereade__: anyway, too early for me to argue properly :) I'll just do it in the provider tests as a helper, which can be extracted if needed [07:37] dimitern, provider or provisioner? :) [07:37] fwereade__: yeah, provisioner [07:37] dimitern, cool, thanks [07:38] * dimitern dives into the review queue [07:41] dimitern: about those test failures on trunk... I run "go test ./..." in a fresh Go environment, against trunk, and I get further now but it's still pretty bad: http://paste.ubuntu.com/5675840/ [07:42] Is that what you're getting? [07:42] * thumper is back [07:43] jtv1: you need mongod installed for the state tests [07:43] Ah damn, forgot about that. Will it work on i386 now? [07:43] jtv1: i think now, unless you build it + ssl [07:44] s/now/not/ [07:44] OK, I'll try it on the cloud. Thanks. [07:45] jtv1: but it seems mongo is not the only problem - some things are still missing from the checkout [07:45] rogpeppe: can you help jtv1 with these issues? ^^ [07:46] jtv1: looks like you might not have mongod installed [07:47] I don't — Dimiter already spotted that one. [07:47] I'm trying it on the cloud now. [07:47] rogpeppe: that's not the only problem though - see the undefined stuff [07:47] It was in a fresh Go environment. [07:48] jtv1: what's your current directory and what's your GOPATH set to? [07:49] My GOPATH is /tmp/go (where I built a fresh environment) [07:49] I softlinked $GOPATH/src/launchpad.net/juju-core to my own checkout of trunk, and was cd'ed into that. [07:50] jtv1: ah, that might be your problem [07:52] The softlink, the working directory, or the combination? [07:52] jtv1: the fact that the working directory isn't directly below $GOPATH [07:53] jtv1: what happens if you do "go test launchpad.net/juju-core/..." ? [07:53] jtv1: you can't softlink GOPATH [07:54] sorry [07:54] the go tool is too smart for symlinks [07:54] Too smart for its own good then :) [07:54] jtv1: it tries to work out the package path by looking at the current working directory [07:55] jtv1: unfortunately there's not enough information for it to do so [07:55] That other command line says "launchpad.net/juju-core/..." matched no packages [07:55] jtv1: i'm surprised it didn't give you an error actually [07:55] jtv1: you can softlink stuff inside $GOPATH though - I have ~/work/go/src/launchpad.net/juju-core -> ~/work/juju-core and it works [07:55] Well I'm getting plenty :) [07:55] That's what I did too. [07:56] dimitern: i'm surprised that works actually [07:57] (actually it's the other direction, ~/work/juju-core is a symlink to $GOPATH/src/launchpad.net/juju-core) [07:57] dimitern: ah, that makes more sense [07:57] t [07:57] davecheney: I didn't softlink GOPATH... just $GOPATH/src/launchpad.net/juju-core [07:58] sorry, symlinks inside GOPATH are a no no [07:58] jtv1: the problem is that when the go tool gets the current working directory, it sees ~/mycheckout, not $GOPATH/src/launchpad.net/juju-core [07:58] jtv1: so it can't work out that you're building launchpad.net/juju-core [08:03] rogpeppe: but it'll work as long as I use the launchpad.net/juju-core/... path? [08:03] jtv1: no; i thought it would, but it doesn't [08:03] :( [08:03] jtv1: (i'm not sure why) [08:03] But then how does that softlink work for you!? [08:03] jtv1: it doesn't [08:03] :( [08:03] jtv1: it works for dimiter because his softlink points *inside* GOPATH [08:04] I wonder if maybe it isn't better to have one Go environment per branch... === mthaddon` is now known as mthaddon [08:06] jtv1: i use cobzr, but you can also use bzr colocation [08:07] mgz: we miss you [08:08] takes too long to sign in to g+ these days... [08:13] mramm2: so you don't feel unloved. PING :) [08:41] davecheney: would you be happy if I made environs.config.DefaultSeries a const string? [08:41] I could make the change and merge [08:41] thumper: +1 [08:50] davecheney: just proposing those changes [08:52] thumper: where does version.CurrentNumber come from ? [08:52] was it added recently ? [08:52] yes, added in that branch [08:52] fwereade__: thanks for the notes on our feature branch! We were just looking at how big the divergence was, and I think this'll be helpful. [08:53] thumper: i can't find it ? [08:53] am i an idiot ? [08:53] davecheney: to of version/version.go [08:53] davecheney: first change in that file [08:53] not an idiot, just blind :) [08:53] ahh, yes, v sorts afrer e [08:53] so, i was right first time :) [09:03] :( [09:03] for some reason merging in trunk breaks it [09:03] * thumper pokes [09:06] jam: your sync-tools changes have broken my branch [09:06] * thumper tries to work out the test [09:06] yup, that was the plugin crashing... [09:07] I won't rejoin, signin takes too long [09:08] thumper: the mongo branch or ? [09:08] jam: yeah, my refactoring... [09:08] which hits some fake tools somehow [09:09] thumper: I set up a dummy env, and then copy tools out of it [09:09] I'm getting some extra tools there... [09:09] but you can't set up a dummy env [09:09] without fake tools [09:09] so I delete them [09:09] so the test has known state [09:09] ah, but not all of them... [09:09] now you need to delete 2 of them, I guess [09:09] right/ [09:09] I'll poke [09:09] isn't there a delete all? [09:09] thumper: no, but you can list, iterate, and delete. [09:10] Storage, has Put, Get, List, Remove [09:11] thumper: line 67 is where yoou can change the "lookup Current" into "list all" [09:11] sync_tools_test.go [09:11] * thumper nods [09:11] technically you should be matching against tool patterns, etc. But in practice, I think you can just nuke everything in the dummy env. [09:11] (may get us in trouble in the future :) [09:12] Say, does anyone know why the tests for launchpad.net/juju-core/cmd/juju might be timing out? [09:12] jtv1: if you are timing out, most likely you aren't running a mongo that is new enough to support ssl [09:12] that was the most common occurance I've encountered. [09:12] jtv1: this is a 600s timeout, right? [09:12] Yes. [09:13] During TestPackage. [09:13] So maybe I should try Raring? [09:13] jtv1: if you have the wrong mongod in your PATH, it doesn't start [09:13] jtv1: you have to run from the PPA or use the tarball [09:13] I just installed the quantal mongod. [09:13] jtv1: the one from Julian? [09:13] No, just the standard one. What PPA should I use? [09:14] jtv1: https://launchpad.net/~juju/+archive/experimental [09:14] that only has raring [09:14] just a sec [09:14] https://launchpad.net/~julian-edwards/+archive/mongodb [09:14] has quantal [09:14] Thanks. [09:15] it looks like raring might come with a default one that is new enough: https://launchpad.net/ubuntu/raring/+source/mongodb/1:2.2.4~rc0-0ubuntu1 [09:15] but I'm not positive [09:15] because the SSL support is contentious because of GPL licensing vs OpenSSL, etc. [09:16] OK, I'll try the PPA first then. I'm also keeping notes so hopefully people can re-use them. Thanks. [09:16] thumper: "constant wouldn't be a string": "const foo string = "stuff" [09:16] jam: yeah, that is what I have done [09:21] jam: the target bucket also needs fixing :( [09:24] thumper: I couldn't find exactly when the bucket gets filled, so I just allowed it to have one extra [09:24] nm, I'm fixing [09:24] It looked like putFakeTools was done on first *connect* [09:24] rather than on creation. [09:25] thumper: the other option is to have a dummy config items that lets you disable auto tools. [09:25] jam: sure... but not now :) [09:32] rogpeppe, it looks like Status has been removed from unitinfo recently? [09:32] mattyw: yes, unfortunately. i'm working on putting it back right now. [09:33] rogpeppe, ok no problem === andrewdeane is now known as awd [09:35] jam: managed to fix all the issues during the call :) [09:35] :) [09:35] just doing a final test run prior to submitting [09:35] jtv1, you're welcome :) [09:35] jtv1, there will certainly be more but I need to look at it in bits :) [09:35] I have a feeling tomorrow will be doing all the peer reviews [09:35] for me at least [09:35] also [09:35] yay for checking the tests prior to merging [09:36] with trunk merged in [09:37] fwereade__: I'm trying to get trunk to pass tests right now, and after that I can get a bit of an overview of the divergence. rvba and I are planning to do the fixing for this one. [09:37] jtv1, cool [09:41] * thumper has merged the branch, and is leaving the office [09:41] ciao [10:12] fwereade__: the provisioner currently aborts if any machine failed to start [10:13] fwereade__: I didn't change that, it was already like this [10:20] dimitern: just FYI, revision 1101 broke our tests against go tip for me. [10:20] rogpeppe: can I see a paste? [10:20] dimitern: it's probably the go runtime's fault, not yours [10:21] dimitern: http://paste.ubuntu.com/5676161/ [10:21] dimitern: that's running TestUniterRelations only [10:22] rogpeppe: I have no idea why [10:23] rogpeppe: could it be a bug in mgo? [10:25] dimitern: i really don't know. i'm trying to narrow down the problem first. [10:27] dimitern: it's not helped by the fact that from go revisions 15910 to 16052 our stuff does not build due to a bug in the go compiler. [10:28] rogpeppe: ha.. interesting [10:35] dimitern, yeah, but making that sane is part of the job, I think [10:35] dimitern, sorry, bbiab [10:44] fwereade__: my point is, when it fails it exits the loop, like the uniter does, and gets restarted [10:53] updated https://codereview.appspot.com/8330043/ [11:04] dimitern, sorry, still half-having lunch, but: the uniter sets a hook error state and keeps going, because it's meant to handle that failure [11:04] dimitern, the situation is analogous [11:05] dimitern, besides, letting the pro die will fuck the firewaller at the same time, and that has even more complex state to set up every time [11:05] fwereade__: so this means general refactoring of the provisioner, not just setting an error status, which I think is outside of this CL [11:06] fwereade__: it'll die exactly the same way now, no regression introduced [11:07] fwereade__: i'm happy to do the refactoring though, in a follow-up - will get me a chance to know it a whole lot better [11:24] 'AaaaaAaa,e. [11:26] jam: very expressive encoding :) [11:26] dimitern: 5-year old keyboard typing. And me hitting instead of [11:29] dimitern, mgz: standup? [11:35] dimitern, I think there has been a communication error [11:36] fwereade__: oh, what was it? [11:37] dimitern, meh, just what I thought I meant and what you thought I meant re the story [11:37] dimitern, it is ofc my fault :) [11:38] fwereade__: sorry, but it's still not clear for me [11:38] dimitern, in my mind, making the provisioner react sensibly to predictable errors -- as does the uniter -- is absolutely part of this story [11:39] dimitern, think through step by step what will happen when we try to provision a machine with bad constraints [11:41] dimitern, the provisioner starts up, sees an unprovisioned machine, tries to provision it... falls over [11:42] dimitern, gets restarted, sees an upprovisioned machine, tries to provision it... falls over [11:42] dimitern, and may well continue doing this forever even if there are other machines that *could* be provisioned [11:44] dimitern, now we have a channel by which errors can escape to something that'll handle them -- however badly -- we should (1) send the errors down that channel and (2) not send them elsewhere [11:44] dimitern, in my mind, the two pieces of work are two sides of the same coin [11:46] fwereade__: wanna have a quick g+? [11:46] dimitern, sure [12:21] TheMue, ping [12:40] fwereade__: pong [12:41] fwereade__: just proposing the changes ;) [12:41] TheMue, heyhey, just wanted to say I'm around if you want to chat about the uuid stuff [12:41] fwereade__: found your feedback clear (at least i hope i got it right ;) ) [12:41] TheMue, cool :) [12:42] fwereade__: so, it's in again [12:42] TheMue, what are the plans for env doc key vs globalKey then? ah ok cool I'll read it :) [12:43] fwereade__: i liked your idea, it's then a real global key. [12:44] TheMue, sorry, I think I failed to be clear here [12:45] fwereade__: i'm listening [12:45] TheMue, the globalKey for the environment was until recently "e" [12:45] TheMue, I'm not sure why that changed on the Environment entity in the first place [12:45] fwereade__: "e#<>" [12:47] TheMue, http://paste.ubuntu.com/5676494/ [12:47] TheMue, the global key for the environment is still "e" [12:47] TheMue, that globalKey() method is I think objectively wrong [12:48] fwereade__: ah, yes, i meant the the method globalKey() returned [12:48] fwereade__: the "e", like for the settings, has been the reason i've used "e" in the first run as document id too [12:48] frankban, ping [12:49] TheMue, yes, exactly [12:49] fwereade__: pong [12:49] frankban, Environment.globalKey() [12:49] frankban, it doesn't match the usual environment global key as used elsewhere; is this because the existing one ("e") is crazy, or because you didn't know about it? [12:50] TheMue, so, whatever we pick for globalKey is irrelevant [12:51] fwereade__: in order for the PA to pick up machines in error state _at all_ is to watch for something other than Life [12:51] fwereade__: OIC, you mean "e#[name]" vs just "e"? if so, the latter. [12:51] fwereade__: I mean machines that were in error status but was resolved [12:51] frankban, cool, thanks, just checking [12:52] dimitern, ah-ha! [12:52] dimitern, ok that makes things more interesting/annoying in the future [12:52] fwereade__: PA uses WatchMachines() which is a lifecycle watcher, not on status or anything else [12:52] dimitern, but for now remember we don't expose any way to resolve [12:52] fwereade__: so globalKey() should only return "e" and that's also ok as document id for the environment doc in its collection? [12:52] dimitern, so it doesn't matter yet [12:52] TheMue, (1) probably (2) maybe [12:53] fwereade__: hehe [12:53] fwereade__: so I cannot test the machine is skipped - it'll always be skipped once created and is in error state [12:53] TheMue, in both cases it's worth a discussion if you're not familiar with the context [12:53] fwereade__: even after restarting the PA the machine still won't be picked up [12:54] dimitern, and that is correct behaviour [12:54] dimitern, verify that it happens and you're good, I think [12:54] fwereade__: ok, but seems incomplete [12:54] fwereade__: as the doc is the only one in it's collection the id only has to be static, so i initially thought "e" like the one for the environment conf or its constraints would be fine. [12:54] fwereade__: but I guess when we have to deal with resolve it'll come back to this [12:55] dimitern, yeah -- I think it's a problem we can safely solve then [12:55] fwereade__: for the globalKey() i have to look more, indeed. interestingly the tests pass all. [12:55] Good mornings [12:55] dimitern: Fixed: https://codereview.appspot.com/8355044 [12:56] TheMue, I think I'm fine with "e" all round, actually, but please make sure it's a const somewhere and not used in magic string form [12:56] niemeyer: cool, thanks I'll take a look [12:57] fwereade__: so shall i change all other places where today simply "e" is used too? [12:57] TheMue, yes please [12:57] fwereade__: will do so [12:57] TheMue, although, wait a mo [12:58] fwereade__: yes, until your review is in [12:58] TheMue, I'm suspicious of having the two keys be the same [12:58] TheMue, any particular reason not to use the uuid? [12:59] TheMue, it *is* a unique identifier for the environment, after all :) [12:59] TheMue, (we should not use the uuid for the global key, just for the entity key) [12:59] fwereade__: when doing the FindId() i have to know the id. otherwise i could also read the first document of the collection. would work too. [13:00] TheMue, just-pick-one is OK for now [13:01] TheMue, if we end up sharing that collection when multitenanting it'll break, but we'd always have to have the env uuid available in the *State, so it'd be a trivial fix [13:03] fwereade__: ok, then will do it this way. so i'll use the uuid field as _id. [13:07] niemeyer: LGTM [13:13] fwereade__: i think this should be about all of it: https://codereview.appspot.com/8330043/ [13:15] dimitern, ack [13:15] * dimitern lunch === sidnei` is now known as sidnei [13:54] I have a small branch up for review, it's pretty straight-forward: https://codereview.appspot.com/8366043 [13:56] * rogpeppe is in a sea of *almost* reproducible bugs and uncertain dependencies [14:03] benji: I'm on it shortly [14:04] thanks dimitern [14:05] mgz: do you know of a way of getting bzr to print the revision id it's currently on (not just the latest revision id it knows about)? [14:05] --tree [14:06] mgz: ah, i missed that, thanks [14:06] mgz: i'm quite surprised it's not the default [14:06] fwereade__: Hi… I just wanted to let you know that I was able to implement the missing method InstanceId() in the MAAS provider. I've followed your advice and used cloudinit to populate a file containing the instanceId and then the method InstanceId() reads from that file. I've just tested my code in the lab with real machines and a real MAAS deployment and it seems to work ok. [14:07] rvba`, thanks, that's great news [14:07] it is a little surprising sometimes === rvba` is now known as rvba [14:08] not an easy default to flip though, revno and revision-info commands mostly used from scripts that would potentially break if the semantics changed [14:09] rvba, I hope it wasn't too much hassle... (there is a path towards completely dropping provider.InstanceId, but it's really nice not to have to do it right now ;)) [14:09] mramm2, rogpeppe, TheMue, hey, should we be meeting? [14:09] fwereade__: already done :D [14:09] fwereade__: ah yes [14:10] TheMue, sorry [14:10] fwereade__, TheMue, mramm2: i'm there [14:10] rogpeppe, fwereade__: we've seen that after this morning and with the current state on the board we don't need the meeting today [14:10] rogpeppe, funny, I think I'm there too [14:10] rogpeppe, moot anyway :) [14:10] fwereade__: it was actually pretty straightforward apart from one trick: I added a method in cloudinit.go to be able to add a runcmd at the *beginning* of the list of commands. [14:11] Because jujud is started by cloudinit and we need the file containing the instanceId to be present before jujud is started. [14:11] rvba, I've been kinda expecting us to need something like that for about a year, so np there :) [14:12] All right :) [14:13] fwereade__, rvba: another possibility would be to change environs/cloudinit so you pass in the cloudinit.Config instead of returning it [14:14] rogpeppe, hmm, that's rather nice I think, and seems to fit the style of cloudinit-composition already used in maas -- rvba, comment? [14:16] fwereade__: which way do you prefer the passing of the UUID to NewHookContext to be tested? it's done implicit by compared what is passed to what later is retrieved as env variable. [14:16] fwereade__: we might rename cloudinit.New to cloudinit.Config.Configure if we did that, perhaps [14:16] fwereade__: rogpeppe sounds like a good idea… [14:17] fwereade__: the field is only used to set the env variable, there's no other accessor [14:17] rvba, then maybe go with that if that's ok? [14:17] TheMue, so, you need to check it in an actual hook [14:17] fwereade__: ok, I'll give it a try. [14:17] rvba: thanks [14:17] TheMue, ATM you don't test that the env var actually has the correct value [14:17] rvba, rogpeppe: tyvm [14:18] fwereade__: i do, with the latest CL [14:18] TheMue, you do not [14:18] fwereade__: it's generated once, passed to NewHookContext and compared with the env variable in AssertEnv() [14:18] TheMue, you test HookContext [14:19] TheMue, you do not ever test the correct value of the environment uuid, for which you need to actually run a uniter [14:20] TheMue, you do test, perfectly well, that a hook context created with a given UUID will put that UUID into the env [14:20] fwereade__: then i got your comment wrong. thought you meant the passing to the HookContext [14:20]