Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

VanillaSoftwareProcess tests #223

Merged
merged 4 commits into from Jul 19, 2016

Conversation

aledsage
Copy link
Contributor

For " Don’t call postInstallCommand if skipInstall=true", why did we previous still call postInstall @grkvlt ?

The bulk of this PR is the unit testing of VanillaSoftwareProcess, using the RecordSshTool to stub out all the ssh commands.

Note the "Adds EffectorUtilsTest" is unrelated - I meant to include it in a PR a while ago, but accidentally missed it! We previously didn't have any tests that covered these methods directly.

@aledsage aledsage force-pushed the VanillaSoftwareProcess-tests branch from 5cc274f to d24e0de Compare June 28, 2016 21:05
@grkvlt
Copy link
Member

grkvlt commented Jul 5, 2016

@aledsage not sure about postInstall() but I wonder if there was an entity that relied on that behaviour in Clocker? I will check before we merge this...

@neykov
Copy link
Member

neykov commented Jul 14, 2016

It was something to do with entities setting object properties on postInstall, later required in the launch step. We got NPEs if postInstall isn't called.

@neykov
Copy link
Member

neykov commented Jul 14, 2016

LGTM

@aledsage-tmp
Copy link

@grkvlt any objections to me merging this? I'll do so later today, unless you say otherwise.

@asfgit asfgit merged commit d24e0de into apache:master Jul 19, 2016
asfgit pushed a commit that referenced this pull request Jul 19, 2016
@aledsage aledsage deleted the VanillaSoftwareProcess-tests branch August 9, 2016 19:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
5 participants