Skip to content

Commit

Permalink
Fix panics on ddev list and ddev start (with no config), fixes #521 (#…
Browse files Browse the repository at this point in the history
…533)

* Make sure there's an existing AppConfig even if it has problems

* Make ddev start work even if no .ddev found

* Add test to make sure start without config works

* Add TestListWithoutDir to make sure listing works even if site dir is missing

* Fix TestListWithoutDir() issues from rebase upstream
  • Loading branch information
rfay committed Nov 8, 2017
1 parent d7b5745 commit d27e94a
Show file tree
Hide file tree
Showing 4 changed files with 97 additions and 6 deletions.
2 changes: 1 addition & 1 deletion cmd/ddev/cmd/list.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,10 +27,10 @@ var DevListCmd = &cobra.Command{
table := platform.CreateAppTable()
for _, site := range sites {
desc, err := site.Describe()
appDescs = append(appDescs, desc)
if err != nil {
util.Failed("Failed to describe site %s: %v", site.GetName(), err)
}
appDescs = append(appDescs, desc)
platform.RenderAppRow(table, desc)
}

Expand Down
2 changes: 1 addition & 1 deletion cmd/ddev/cmd/start.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ provide a working environment for development.`,
func appStart() {
app, err := platform.GetActiveApp("")
if err != nil {
util.Failed("Failed to start %s, err: %v", app.GetName(), err)
util.Failed("Failed to start: %v", err)
}

output.UserOut.Printf("Starting environment for %s...", app.GetName())
Expand Down
9 changes: 5 additions & 4 deletions pkg/plugins/platform/local.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,9 +45,12 @@ func (l *LocalApp) GetType() string {
// Init populates LocalApp settings based on the current working directory.
func (l *LocalApp) Init(basePath string) error {
config, err := ddevapp.NewConfig(basePath, "")

// Save config to l.AppConfig so we can capture and display the site's
// status regardless of its validity
l.AppConfig = config

if err != nil {
// Save config to l.AppConfig so we can capture and display the site's status.
l.AppConfig = config
return err
}

Expand All @@ -56,8 +59,6 @@ func (l *LocalApp) Init(basePath string) error {
return err
}

l.AppConfig = config

web, err := l.FindContainerByType("web")
if err == nil {
containerApproot := web.Labels["com.ddev.approot"]
Expand Down
90 changes: 90 additions & 0 deletions pkg/plugins/platform/local_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -177,6 +177,27 @@ func TestLocalStart(t *testing.T) {
testcommon.CleanupDir(another.Dir)
}

// TestStartWithoutDdev makes sure we don't have a regression where lack of .ddev
// causes a panic.
func TestStartWithoutDdevConfig(t *testing.T) {
// Set up tests and give ourselves a working directory.
assert := asrt.New(t)
testDir := testcommon.CreateTmpDir("TestStartWithoutDdevConfig")

// testcommon.Chdir()() and CleanupDir() check their own errors (and exit)
defer testcommon.CleanupDir(testDir)
defer testcommon.Chdir(testDir)()

err := os.MkdirAll(testDir+"/sites/default", 0777)
assert.NoError(err)
err = os.Chdir(testDir)
assert.NoError(err)

_, err = platform.GetActiveApp("")
assert.Error(err)
assert.Contains(err.Error(), "unable to determine")
}

// TestGetApps tests the GetApps function to ensure it accurately returns a list of running applications.
func TestGetApps(t *testing.T) {
assert := asrt.New(t)
Expand Down Expand Up @@ -619,6 +640,75 @@ func TestRouterNotRunning(t *testing.T) {
}
}

// TestListWithoutDir prevents regression where ddev list panics if one of the
// sites found is missing a directory
func TestListWithoutDir(t *testing.T) {
// Set up tests and give ourselves a working directory.
assert := asrt.New(t)
testcommon.ClearDockerEnv()
packageDir, _ := os.Getwd()

// startCount is the count of apps at the start of this adventure
apps := platform.GetApps()
startCount := len(apps["local"])

testDir := testcommon.CreateTmpDir("TestStartWithoutDdevConfig")
defer testcommon.CleanupDir(testDir)

err := os.MkdirAll(testDir+"/sites/default", 0777)
assert.NoError(err)
err = os.Chdir(testDir)
assert.NoError(err)

config, err := ddevapp.NewConfig(testDir, "")
assert.NoError(err)
config.Name = "junk"
config.AppType = "drupal7"
err = config.Write()
assert.NoError(err)

// Do a start on the configured site.
app, err := platform.GetActiveApp("")
assert.NoError(err)
err = app.Start()
assert.NoError(err)

// Make sure we move out of the directory for Windows' sake
garbageDir := testcommon.CreateTmpDir("RestingHere")
defer testcommon.CleanupDir(garbageDir)

err = os.Chdir(garbageDir)
assert.NoError(err)

testcommon.CleanupDir(testDir)

apps = platform.GetApps()

assert.EqualValues(len(apps["local"]), startCount+1)

// Make a whole table and make sure our app directory missing shows up.
// This could be done otherwise, but we'd have to go find the site in the
// array first.
table := platform.CreateAppTable()
for _, site := range apps["local"] {
desc, err := site.Describe()
if err != nil {
t.Fatalf("Failed to describe site %s: %v", site.GetName(), err)
}

platform.RenderAppRow(table, desc)
}
assert.Contains(table.String(), fmt.Sprintf("app directory missing: %s", testDir))

err = app.Down(true)
assert.NoError(err)

// Change back to package dir. Lots of things will have to be cleaned up
// in defers, and for windows we have to not be sitting in them.
err = os.Chdir(packageDir)
assert.NoError(err)
}

// constructContainerName builds a container name given the type (web/db/dba) and the app
func constructContainerName(containerType string, app platform.App) (string, error) {
container, err := app.FindContainerByType(containerType)
Expand Down

0 comments on commit d27e94a

Please sign in to comment.