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.

Sign up for GitHub

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

Jump to bottom

MESOS: flocker unit test should clean up after itself #15158

Merged
merged 1 commit into from
Oct 10, 2015
Merged

MESOS: flocker unit test should clean up after itself #15158

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
21 changes: 14 additions & 7 deletions pkg/volume/flocker/plugin_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@ limitations under the License.
package flocker

import (
"io/ioutil"
"os"
"testing"

flockerClient "github.com/ClusterHQ/flocker-go"
Expand All @@ -28,15 +30,17 @@ import (

const pluginName = "kubernetes.io/flocker"

func newInitializedVolumePlugMgr() volume.VolumePluginMgr {
func newInitializedVolumePlugMgr(t *testing.T) (volume.VolumePluginMgr, string) {
plugMgr := volume.VolumePluginMgr{}
plugMgr.InitPlugins(ProbeVolumePlugins(), volume.NewFakeVolumeHost("/foo/bar", nil, nil))
return plugMgr
dir, err := ioutil.TempDir("", "flocker")
assert.NoError(t, err)
plugMgr.InitPlugins(ProbeVolumePlugins(), volume.NewFakeVolumeHost(dir, nil, nil))
return plugMgr, dir
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if there is a better way of getting this dir than returning it here (it's not being used in 3 out of 4 functions). Perhaps with the builder created in line 212? There is no problem doing the cleanup there because the function SetupAt was not called yet.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Honestly I'd rather return the dir here and just ignore it when not needed
vs. incorporating logic into the unit test that reaches into the bowels of
a structure to extract the path some other way. This seems much more
straightforward.

On Wed, Oct 7, 2015 at 10:44 AM, Alexandre González <
notifications@github.com> wrote:

In pkg/volume/flocker/plugin_test.go
#15158 (comment)
:

plugMgr := volume.VolumePluginMgr{}
  • plugMgr.InitPlugins(ProbeVolumePlugins(), volume.NewFakeVolumeHost("/foo/bar", nil, nil))
  • return plugMgr
  • dir, err := ioutil.TempDir("", "flocker")
  • assert.NoError(t, err)
  • plugMgr.InitPlugins(ProbeVolumePlugins(), volume.NewFakeVolumeHost(dir, nil, nil))
  • return plugMgr, dir

I wonder if there is a better way of getting this dir than returning it
here (it's not being used in 3 out of 4 functions). Perhaps with the
builder created in line 212? There is no problem doing the cleanup there
because the function SetupAt was not called yet.

—
Reply to this email directly or view it on GitHub
https://github.com/kubernetes/kubernetes/pull/15158/files#r41398502.

}

func TestGetByName(t *testing.T) {
assert := assert.New(t)
plugMgr := newInitializedVolumePlugMgr()
plugMgr, _ := newInitializedVolumePlugMgr(t)

plug, err := plugMgr.FindPluginByName(pluginName)
assert.NotNil(plug, "Can't find the plugin by name")
Expand All @@ -45,7 +49,7 @@ func TestGetByName(t *testing.T) {

func TestCanSupport(t *testing.T) {
assert := assert.New(t)
plugMgr := newInitializedVolumePlugMgr()
plugMgr, _ := newInitializedVolumePlugMgr(t)

plug, err := plugMgr.FindPluginByName(pluginName)
assert.NoError(err)
Expand Down Expand Up @@ -113,7 +117,7 @@ func TestGetFlockerVolumeSource(t *testing.T) {
func TestNewBuilder(t *testing.T) {
assert := assert.New(t)

plugMgr := newInitializedVolumePlugMgr()
plugMgr, _ := newInitializedVolumePlugMgr(t)
plug, err := plugMgr.FindPluginByName(pluginName)
assert.NoError(err)

Expand Down Expand Up @@ -196,7 +200,10 @@ func TestSetUpAtInternal(t *testing.T) {

assert := assert.New(t)

plugMgr := newInitializedVolumePlugMgr()
plugMgr, rootDir := newInitializedVolumePlugMgr(t)
if rootDir != "" {
defer os.RemoveAll(rootDir)
}
plug, err := plugMgr.FindPluginByName(flockerPluginName)
assert.NoError(err)

Expand Down