tests: added template + metadata functional test #1512
Conversation
kayrus
force-pushed
the
kayrus/template_specifiers
branch
from
f7ea93b to
eca5000
Compare
|
@jonboulle reverting of PR #1376 resolved the issue. We can keep these tests and merge this PR. Or I can spend more time and fix initial PR. What is your decision? |
|
#1376 did not make it into a release right? If not, let's revert that + add the test for this release, and then we can rework a proper version for the next release. Thanks! |
kayrus
changed the title
|
@jonboulle no, it didn't. tests cover the bug. new #1514 issue has been created. waiting for LGTM ;) |
| ndesired := 3 | ||
| if stdout, stderr, err := cluster.Fleetctl(m0, "list-units", "--no-legend", "--full", "--fields", "unit,active,machine"); err != nil { | ||
| t.Fatalf("Unable to get submited units: \nstdout: %s\nstderr: %s\nerr: %v", stdout, stderr, err) | ||
| } else { |
There was a problem hiding this comment.
Remove the else - Fatal will exit anyway
|
@jonboulle Done. |
kayrus
force-pushed
the
kayrus/template_specifiers
branch
from
f1afb73 to
6d873b4
Compare
|
@jonboulle I discussed this with @kayrus, so maybe spend some time on it? cause it's really an optimization boost, previous logic triggered unnecessary RPC and I/O work. The code that can be fixed was located, and there is already a functional test for it, otherwise I'm ok. |
|
I think you're right. |
|
@kayrus some formal nitpicks: first of all, please update the title of this PR -- it doesn't apply anymore, and it's very confusing... I think the new tests should be applied after the revert, to preserve bisectability. Also, this might be a matter of taste -- but I believe it would be better to have only one revert commit covering both of the original commits (i.e. using |
|
Meh, too late ;-) |
|
Sorry, all fair comments, just wanted to unblock. On Wed, Mar 23, 2016 at 4:09 PM Olaf Buddenhagen notifications@github.com
|
Covers #1446