Resolving the same windows test failures again

[Nate Finch]

Sorry for the lack of tests for that part of the code, I guess I added them
for one and not the other.   Note that I had actually been leaving that PR
unmerged because I knew master was blocked.  Sorry it got pushed in anyway,
and caused problems.

On Mon, May 4, 2015 at 7:20 PM, Martin Packman <martin.packman at canonical.com
> wrote:

> There was some confusion about the regression to the windows test
> failures on trunk.
>
> <https://bugs.launchpad.net/juju-core/+bug/1450919>
>
> Partly my fault, Curtis initially looked at the 1.24 branch and I
> looked at trunk, and each branch has a different issue. Here's what
> I've just done to diagnose.
>
>
> So, trunk first, I noticed the failures looked exactly the same as
> before CI fiddled with the environment variable casing to work around
> a juju bug:
>
> <https://launchpad.net/bugs/1446871>
>
> The fix for this bug landed on master, therefore it's easy to assume
> that change actually broke the casing behaviour in such a way as to
> invalidate the CI hack, rather than fixing the underlying issue.
>
> Looking at the code:
>
> <https://github.com/juju/juju/pull/2124/files>
>
> Two similar functions now exist for merging case, mergeEnvWin which
> works on map[string]string and mergeWindowsEnvironment which works on
> []string. Only mergeEnvWin has tests. Guess, mergeWindowsEnvironment
> is bugged. Indeed:
>
> -               m[strings.ToUpper(varSplit[0])] = varSplit[1]
> +               k := varSplit[0]
> +               if _, ok := uppers[strings.ToUpper(k)]; ok {
> +                       uppers[k] = varSplit[1]
>
> It's not uppercasing the key in the assignment. So, Path is matched to
> PATH, Path is assigned to, but later only PATH is pulled out.
>
>
> Now for the 1.24 branch, no hints here. So, lets see what changed.
> Latest working 1.24 without windows test failures:
>
> <http://reports.vapour.ws/releases/2581>
>
> Earliest 1.24 with windows test failures:
>
> <http://reports.vapour.ws/releases/2592>
>
> $ git diff fffe3e4f..95e7619f
>
> 2770 lines... Not helpful. But, error mentions "cannot move the charm
> archive" and:
>
> -gopkg.in/juju/charm.v5 git
> 6b74a2771545912f8a91a544b0f28405b9938624        2015-04-14T14:33:47Z
> +gopkg.in/juju/charm.v5 git
> 779394167ac61b02ca73ca17c3012f05a5ba316c        2015-04-30T02:46:55Z
>
> $ pushd ~/go/src/gopkg.in/juju/charm.v5
>
> Try to update and branches diverged? Wha?
>
> $ git diff 6b74a277..77939416
>
> Er... this includes a revert of my licence header changes, and, the
> answer to the test failures, Gabriel's file closing fix:
> <https://github.com/juju/charm/pull/119>
> <https://github.com/juju/charm/pull/120>
>
> So, just renaming a variable is *not* safe (if you accidentally back
> out other changes when merging).
>
>
> Both these regressions happened due to landing 'safe' changes while
> the branch was broken, and were therefore harder to pin down than they
> would otherwise have been.
>
> Martin
>
> --
> Juju-dev mailing list
> Juju-dev at lists.ubuntu.com
> Modify settings or unsubscribe at:
> https://lists.ubuntu.com/mailman/listinfo/juju-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.ubuntu.com/archives/juju-dev/attachments/20150504/e29d89f8/attachment.html>