maas-test

Merge lp://qastaging/~rvb/maas-test/apache-timeout into lp://qastaging/maas-test

apache-timeout
Merge into trunk

Proposed by Raphaël Badin on 2013-12-17

Status:

Merged

Approved by:

Raphaël Badin on 2013-12-17

Approved revision:

Merged at revision:

Proposed branch:

lp://qastaging/~rvb/maas-test/apache-timeout

Merge into:

lp://qastaging/maas-test

Diff against target:

206 lines (+116/-12)

4 files modified

maastest/maasfixture.py (+10/-2)
maastest/tests/test_maasfixture.py (+78/-4)
maastest/tests/test_utils.py (+17/-0)
maastest/utils.py (+11/-6)

To merge this branch:

bzr merge lp://qastaging/~rvb/maas-test/apache-timeout

High

Fix Committed

Link a bug report

Reviewer	Review Type	Date Requested	Status
Jeroen T. Vermeulen (community)		2013-12-17	Approve on 2013-12-17
Review via email: mp+199257@code.qastaging.launchpad.net

Commit message

Try to restart apache2 a couple of time before giving up.

Revision history for this message

Jeroen T. Vermeulen (jtv) wrote on 2013-12-17:

It's a shame we can't get to the root cause right now, but it does look as if it may be in the Apache package somewhere...

A few notes as always:

An alternative way to make tests ignore the "rewrite configuration" command might be to extract that command into a separate function, and have the test patch out that function. Probably not worth it here though, but it's a recurring problem when invoking shell commands.

I think test_configure_default_maas_url_fails_apache_cannot_restart is missing an "if" in the name. Should still just barely fit on a line!

Perhaps make_exception() could decode as UTF-8? It's a reasonable guess, and text that isn't UTF-8 is highly unlikely to decode cleanly as UTF-8. The disadvantage would be that non-ASCII errors might be handled well only as long as they were in UTF-8, which might end up hiding mistakes from us or confronting people far away with more difficult errors than we see.

review: Approve

lp://qastaging/~rvb/maas-test/apache-timeout updated on 2013-12-17

98. By Raphaël Badin on 2013-12-17: Review fixes.

Revision history for this message

Raphaël Badin (rvb) wrote on 2013-12-17:

> It's a shame we can't get to the root cause right now, but it does look as if
> it may be in the Apache package somewhere...
>
> A few notes as always:
>
> .
>
> An alternative way to make tests ignore the "rewrite configuration" command
> might be to extract that command into a separate function, and have the test
> patch out that function. Probably not worth it here though, but it's a
> recurring problem when invoking shell commands.

Yeah, I thought about that but keeping the rewrite of the conf and the restart of apache under the same roof (i.e. in the same method) seemed to make more sense to me rather that refactor it for the sake of testing.

> I think test_configure_default_maas_url_fails_apache_cannot_restart is missing
> an "if" in the name. Should still just barely fit on a line!

Right, done.

> Perhaps make_exception() could decode as UTF-8? It's a reasonable guess, and
> text that isn't UTF-8 is highly unlikely to decode cleanly as UTF-8. The
> disadvantage would be that non-ASCII errors might be handled well only as long
> as they were in UTF-8, which might end up hiding mistakes from us or
> confronting people far away with more difficult errors than we see.

Good point, fixed.