Merge lp:~free.ekanayaka/fake-juju/initial-unit-tests into lp:~landscape/fake-juju/trunk-old

[Free Ekanayaka]

This branch is the first of a series that will refactor the fake-juju Go code to make it more understandable, maintainable and testable.

As first step, I've added initial 2.0.2 support (not yet fully working) and re-organized a bit of the existing fake-juju.go code into unit-tested modules, which you can find under common/service.

The idea is that each individual version-specific directory will only override what needs to change for that specific version, and code under common/ will be shared.

You can run the unit tests with:

make -C 2.0.2 unit-test

and build/run this WIP fake-juju with:

make -C 2.0.2 build
./2.0.2/2.0.2

You'll see some output and then the process will exit. More behavior will come.

[🤖 Landscape Builder (landscape-builder)]

review: Abstain (executing tests)

[lp:~free.ekanayaka/fake-juju/initial-unit-tests]

61. By Free Ekanayaka on 2016-11-29

Skip regular tests

[🤖 Landscape Builder (landscape-builder)]

Command: make ci-test
Result: Fail
Revno: 60
Branch: lp:~free.ekanayaka/fake-juju/initial-unit-tests
Jenkins: https://ci.lscape.net/job/latch-test-xenial/1032/

review: Needs Fixing (test results)

[🤖 Landscape Builder (landscape-builder)]

review: Abstain (executing tests)

[lp:~free.ekanayaka/fake-juju/initial-unit-tests]

62. By Free Ekanayaka on 2016-11-29

Add missing import

[🤖 Landscape Builder (landscape-builder)]

Command: make ci-test
Result: Fail
Revno: 61
Branch: lp:~free.ekanayaka/fake-juju/initial-unit-tests
Jenkins: https://ci.lscape.net/job/latch-test-xenial/1033/

review: Needs Fixing (test results)

[🤖 Landscape Builder (landscape-builder)]

review: Abstain (executing tests)

[🤖 Landscape Builder (landscape-builder)]

Command: make ci-test
Result: Success
Revno: 62
Branch: lp:~free.ekanayaka/fake-juju/initial-unit-tests
Jenkins: https://ci.lscape.net/job/latch-test-xenial/1034/

review: Approve (test results)

[lp:~free.ekanayaka/fake-juju/initial-unit-tests]

63. By Free Ekanayaka on 2016-11-30

Rename FakeJujuState to FakeJujuService

[🤖 Landscape Builder (landscape-builder)]

review: Abstain (executing tests)

[🤖 Landscape Builder (landscape-builder)]

Command: make ci-test
Result: Success
Revno: 63
Branch: lp:~free.ekanayaka/fake-juju/initial-unit-tests
Jenkins: https://ci.lscape.net/job/latch-test-xenial/1035/

review: Approve (test results)

[lp:~free.ekanayaka/fake-juju/initial-unit-tests]

64. By Free Ekanayaka on 2016-11-30

Don't let LoggingSuite reset logging

[🤖 Landscape Builder (landscape-builder)]

review: Abstain (executing tests)

[🤖 Landscape Builder (landscape-builder)]

Command: make ci-test
Result: Fail
Revno: 64
Branch: lp:~free.ekanayaka/fake-juju/initial-unit-tests
Jenkins: https://ci.lscape.net/job/latch-test-xenial/1036/

review: Needs Fixing (test results)

[lp:~free.ekanayaka/fake-juju/initial-unit-tests]

65. By Free Ekanayaka on 2016-11-30

Fix patch file

[🤖 Landscape Builder (landscape-builder)]

review: Abstain (executing tests)

[🤖 Landscape Builder (landscape-builder)]

Command: make ci-test
Result: Success
Revno: 65
Branch: lp:~free.ekanayaka/fake-juju/initial-unit-tests
Jenkins: https://ci.lscape.net/job/latch-test-xenial/1037/

review: Approve (test results)

[Francis Ginther (fginther)]

Compared this with the existing 2.0.0 version and the refactoring so far appears reasonable. As stated, much is still missing, but the refactoring approach is cleaner and more readable. And yay unit-tests!

Someone with more experience working on fake-juju may find a few things to comment on, but this looks good from my perspective.

review: Approve

=== added directory '2.0.2'
=== added file '2.0.2/Makefile'
--- 2.0.2/Makefile	1970-01-01 00:00:00 +0000
+++ 2.0.2/Makefile	2016-11-30 06:28:02 +0000
@@ -0,0 +1,5 @@
+include ../common/makefile.mk
+
+# XXX Functionality is not complete yet
+test:
+	echo "Skip, fake-juju 2.0.2 is WIP"

=== added file '2.0.2/fake-juju.go'
--- 2.0.2/fake-juju.go	1970-01-01 00:00:00 +0000
+++ 2.0.2/fake-juju.go	2016-11-30 06:28:02 +0000
@@ -0,0 +1,12 @@
+package main
+
+import (
+	"os"
+	"./service"
+)
+
+func main() {
+	code := 0
+	service.RunFakeJuju()
+	os.Exit(code)
+}

=== added file '2.0.2/juju-core.patch'
--- 2.0.2/juju-core.patch	1970-01-01 00:00:00 +0000
+++ 2.0.2/juju-core.patch	2016-11-30 06:28:02 +0000
@@ -0,0 +1,101 @@
+diff -U 3 -r --no-dereference .unpacked-clean/github.com/juju/juju/apiserver/charms.go src/github.com/juju/juju/apiserver/charms.go
+--- .unpacked-clean/github.com/juju/juju/apiserver/charms.go	2016-11-11 00:08:09.000000000 +0000
++++ src/github.com/juju/juju/apiserver/charms.go	2016-11-30 06:09:26.975210508 +0000
+@@ -284,7 +284,8 @@
+ 		if isImporting, err := modelIsImporting(st); err != nil {
+ 			return nil, errors.Trace(err)
+ 		} else if !isImporting {
+-			return nil, errors.New("cs charms may only be uploaded during model migration import")
++			// fake-juju allows uploading "charmstore" charms.
++			//return nil, errors.New("cs charms may only be uploaded during model migration import")
+ 		}
+ 
+ 		// If a revision argument is provided, it takes precedence
+diff -U 3 -r --no-dereference .unpacked-clean/github.com/juju/juju/provider/dummy/environs.go src/github.com/juju/juju/provider/dummy/environs.go
+--- .unpacked-clean/github.com/juju/juju/provider/dummy/environs.go	2016-11-11 00:08:09.000000000 +0000
++++ src/github.com/juju/juju/provider/dummy/environs.go	2016-11-30 06:09:26.977210528 +0000
+@@ -643,9 +643,9 @@
+ 
+ // PrecheckInstance is specified in the state.Prechecker interface.
+ func (*environ) PrecheckInstance(series string, cons constraints.Value, placement string) error {
+-	if placement != "" && placement != "valid" {
+-		return fmt.Errorf("%s placement is invalid", placement)
+-	}
++//	if placement != "" && placement != "valid" {
++//		return fmt.Errorf("%s placement is invalid", placement)
++//	}
+ 	return nil
+ }
+ 
+diff -U 3 -r --no-dereference .unpacked-clean/github.com/juju/juju/testcharms/charm.go src/github.com/juju/juju/testcharms/charm.go
+--- .unpacked-clean/github.com/juju/juju/testcharms/charm.go	2016-11-11 00:08:09.000000000 +0000
++++ src/github.com/juju/juju/testcharms/charm.go	2016-11-30 06:09:26.978210538 +0000
+@@ -17,7 +17,9 @@
+ )
+ 
+ // Repo provides access to the test charm repository.
+-var Repo = testing.NewRepo("charm-repo", "quantal")
++// XXX fake-juju: avoid crashing because the charm-repo dir is not there
++//var Repo = testing.NewRepo("charm-repo", "quantal")
++var Repo = &testing.Repo{}
+ 
+ // UploadCharmWithMeta pushes a new charm to the charmstore.
+ // The uploaded charm takes the supplied charmURL with metadata.yaml and metrics.yaml
+diff -U 3 -r --no-dereference .unpacked-clean/github.com/juju/juju/testing/cert.go src/github.com/juju/juju/testing/cert.go
+--- .unpacked-clean/github.com/juju/juju/testing/cert.go	2016-11-11 00:08:09.000000000 +0000
++++ src/github.com/juju/juju/testing/cert.go	2016-11-30 06:09:26.979210548 +0000
+@@ -52,7 +52,7 @@
+ }
+ 
+ func mustNewCA() (string, string) {
+-	cert.KeyBits = 512
++	cert.KeyBits = 1024
+ 	caCert, caKey, err := cert.NewCA("juju testing", "1234-ABCD-IS-NOT-A-REAL-UUID", time.Now().AddDate(10, 0, 0))
+ 	if err != nil {
+ 		panic(err)
+@@ -61,7 +61,7 @@
+ }
+ 
+ func mustNewServer() (string, string) {
+-	cert.KeyBits = 512
++	cert.KeyBits = 1024
+ 	var hostnames []string
+ 	srvCert, srvKey, err := cert.NewServer(CACert, CAKey, time.Now().AddDate(10, 0, 0), hostnames)
+ 	if err != nil {
+diff -U 3 -r --no-dereference .unpacked-clean/github.com/juju/testing/log.go src/github.com/juju/testing/log.go
+--- .unpacked-clean/github.com/juju/testing/log.go	2016-11-11 00:08:46.000000000 +0000
++++ src/github.com/juju/testing/log.go	2016-11-30 06:09:29.817238914 +0000
+@@ -50,6 +50,9 @@
+ }
+ 
+ func (s *LoggingSuite) TearDownSuite(c *gc.C) {
++	// XXX fake-juju: we don't want this suite to override our logging
++	//     setup.
++	return
+ 	loggo.ResetLogging()
+ }
+ 
+@@ -66,6 +69,10 @@
+ }
+ 
+ func (s *LoggingSuite) setUp(c *gc.C) {
++	// XXX fake-juju: we don't want this suite to override our logging
++	//     setup.
++	return
++
+ 	loggo.ResetLogging()
+ 	// Don't use the default writer for the test logging, which
+ 	// means we can still get logging output from tests that
+diff -U 3 -r --no-dereference .unpacked-clean/gopkg.in/mgo.v2/bson/json.go src/gopkg.in/mgo.v2/bson/json.go
+--- .unpacked-clean/gopkg.in/mgo.v2/bson/json.go	2016-11-11 00:08:13.000000000 +0000
++++ src/gopkg.in/mgo.v2/bson/json.go	2016-11-30 06:09:26.979210548 +0000
+@@ -317,7 +317,8 @@
+ func jencInt(v interface{}) ([]byte, error) {
+ 	n := v.(int)
+ 	f := `{"$numberLong":"%d"}`
+-	if n <= 1<<53 {
++	// fake-juju apply https://github.com/go-mgo/mgo/pull/311
++	if int64(n) <= 1<<53 {
+ 		f = `%d`
+ 	}
+ 	return fbytes(f, n), nil

=== added symlink '2.0.2/service'
=== target is u'../common/service/'
=== modified file 'Makefile'
--- Makefile	2016-11-28 12:56:08 +0000
+++ Makefile	2016-11-30 06:28:02 +0000
@@ -1,4 +1,4 @@
-JUJU_VERSIONS = 1.25.6 2.0.0
+JUJU_VERSIONS = 1.25.6 2.0.0 2.0.2
 
 JUJUCLIENT_DOWNLOADS = $(shell pwd)/tests/jujuclient-archive
 JUJUCLIENT_REQ = $(JUJUCLIENT_DOWNLOADS)/requirements

=== modified file 'common/makefile.mk'
--- common/makefile.mk	2016-11-28 14:10:57 +0000
+++ common/makefile.mk	2016-11-30 06:28:02 +0000
@@ -26,6 +26,10 @@
 	patch -p0 < $(JUJU_PATCH)
 	GOPATH=$(GO_PATH) PATH=$(PATH) go build
 
+.PHONY: unit-test
+unit-test: $(JUJU_TARBALL) $(JUJU_PATCH)
+	GOPATH=$(GO_PATH) go test ./service -gocheck.v
+
 .PHONY: test
 test: $(JUJU_VERSION)
 	cd .. && JUJU_VERSION=$(JUJU_VERSION) python3 -m unittest tests.test_fake

=== added directory 'common/service'
=== added file 'common/service/logging.go'
--- common/service/logging.go	1970-01-01 00:00:00 +0000
+++ common/service/logging.go	2016-11-30 06:28:02 +0000
@@ -0,0 +1,20 @@
+// Logging configuration
+
+package service
+
+import (
+	"io"
+	"github.com/juju/loggo"
+)
+
+var (
+	log = loggo.GetLogger("")
+)
+
+func setupLogging(output io.Writer, level loggo.Level) {
+
+	loggo.ResetWriters()
+	loggo.RegisterWriter("default", loggo.NewColorWriter(output))
+
+	log.SetLogLevel(level)
+}

=== added file 'common/service/run.go'
--- common/service/run.go	1970-01-01 00:00:00 +0000
+++ common/service/run.go	2016-11-30 06:28:02 +0000
@@ -0,0 +1,84 @@
+// Run the fake-juju service
+
+package service
+
+import (
+	"io/ioutil"
+	"os"
+
+	gc "gopkg.in/check.v1"
+
+	"github.com/juju/loggo"
+
+	coretesting "github.com/juju/juju/testing"
+	jujutesting "github.com/juju/testing"
+)
+
+// Main entry point for running the fake-juju service. It will create
+// a FakeJujuSuite instance (yes, a gocheck test suite, see its docstring)
+// and run it (i.e. invoke its single TestStart test method, which will
+// spin a FakeJujuService indefinitely).
+func RunFakeJuju() {
+
+	series := "xenial"
+	if os.Getenv("DEFAULT_SERIES") != "" {
+		series = os.Getenv("DEFAULT_SERIES")
+	}
+
+	options := &FakeJujuOptions{
+		Output: os.Stdout,
+		Level:  loggo.INFO,
+		Series: series,
+		Mongo:  true,
+	}
+	suite := &FakeJujuSuite{
+		options: options,
+	}
+	runner := NewFakeJujuRunner(suite, options)
+	runner.Run()
+}
+
+func NewFakeJujuRunner(suite interface{}, options *FakeJujuOptions) *FakeJujuRunner {
+	return &FakeJujuRunner{
+		suite:   suite,
+		options: options,
+	}
+}
+
+type FakeJujuRunner struct {
+	suite   interface{} // A FakeJujuSuite instance
+	options *FakeJujuOptions
+}
+
+func (f *FakeJujuRunner) Run() *gc.Result {
+
+	setupLogging(f.options.Output, f.options.Level)
+	log.Infof("Starting service")
+
+	// Conditionally start mongo (we don't want this for unit tests).
+	if f.options.Mongo {
+		certs := coretesting.Certs
+		if err := jujutesting.MgoServer.Start(certs); err != nil {
+			return &gc.Result{RunError: err}
+		}
+		defer jujutesting.MgoServer.Destroy()
+	}
+
+	conf := &gc.RunConf{
+		Output: ioutil.Discard, // We don't want any output from gocheck
+		Filter: "TestStart",
+	}
+	result := gc.Run(f.suite, conf)
+
+	if !(result.Succeeded == 1) {
+		message := "No error message"
+		if result.RunError != nil {
+			message = result.RunError.Error()
+		}
+		log.Infof("Service exited uncleanly: %s", message)
+	}
+	log.Infof("Stopping service")
+
+	return result
+
+}

=== added file 'common/service/run_test.go'
--- common/service/run_test.go	1970-01-01 00:00:00 +0000
+++ common/service/run_test.go	2016-11-30 06:28:02 +0000
@@ -0,0 +1,46 @@
+package service_test
+
+import (
+	"bytes"
+
+	gc "gopkg.in/check.v1"
+
+	"github.com/juju/loggo"
+
+	"../service"
+)
+
+type DummySuite struct {
+	options *service.FakeJujuOptions
+	hasRun  bool
+}
+
+func (s *DummySuite) TestStart(c *gc.C) {
+	s.hasRun = true
+}
+
+type FakeJujuRunnerSuite struct {}
+
+// The FakeJujuRunner.Run method executes the given gocheck suite. Logging is
+// configured too.
+func (s *FakeJujuRunnerSuite) TestRun(c *gc.C) {
+	output := &bytes.Buffer{}
+	options := &service.FakeJujuOptions{
+		Output: output,
+		Level:  loggo.DEBUG,
+		Mongo:  false,
+	}
+	suite := &DummySuite{
+		options: options,
+	}
+	runner := service.NewFakeJujuRunner(gc.Suite(suite), options)
+	result := runner.Run()
+
+	c.Assert(result.Succeeded, gc.Equals, 1)
+	c.Assert(result.RunError, gc.IsNil)
+	c.Assert(suite.hasRun, gc.Equals, true)
+	c.Assert(
+		bytes.Contains(output.Bytes(), []byte("Starting service")), gc.Equals, true)
+}
+
+var _ = gc.Suite(&FakeJujuRunnerSuite{})

=== added file 'common/service/service.go'
--- common/service/service.go	1970-01-01 00:00:00 +0000
+++ common/service/service.go	2016-11-30 06:28:02 +0000
@@ -0,0 +1,101 @@
+package service
+
+import (
+	"fmt"
+	"io"
+
+	"github.com/juju/juju/api"
+	"github.com/juju/juju/instance"
+	"github.com/juju/juju/network"
+	"github.com/juju/juju/state"
+	"github.com/juju/juju/version"
+	"github.com/juju/loggo"
+	"github.com/juju/utils"
+
+	semversion "github.com/juju/version"
+)
+
+// Runtime options for the fake-juju service
+type FakeJujuOptions struct {
+	Output io.Writer   // Where to write logs
+	Level  loggo.Level // The log level
+	Series string      // Default Ubuntu series
+	Mongo  bool        // Whether to start mongo, it's off for unit tests
+}
+
+// The core fake-juju service
+func NewFakeJujuService(
+	state *state.State, api api.Connection, options *FakeJujuOptions) *FakeJujuService {
+
+	return &FakeJujuService{
+		state:   state,
+		api:     api,
+		options: options,
+	}
+}
+
+type FakeJujuService struct {
+	state         *state.State
+	api           api.Connection
+	options       *FakeJujuOptions
+	instanceCount int
+}
+
+// Main initialization entry point
+func (s *FakeJujuService) Initialize() error {
+
+	// Juju needs internet access to reach the charm store.  This is
+	// necessary to download charmstore charms (e.g. when adding a
+	// service.  In the case of downloading charms, the controller
+	// does not try to download if the charm is already in the database.
+	// So internet access could go back to disabled if we were to add
+	// the charm directly before using any API methods that try to
+	// download it.  Here are some ideas on how to do that:
+	//  * use the API's "upload charm" HTTP endpoint (POST on /charms);
+	//    this requires disabling the prohibition against uploading
+	//    charmstore charms (which we've done already in fake-juju);
+	//    this could be accomplished using txjuju, python-jujuclient,
+	//    or manually with httplib (all have auth complexity)
+	//  * add an "upload-charm" command to fake-juju that forces a
+	//    charmstore charm into the DB
+	//  * add the service with a "local" charm schema and then forcibly
+	//    change the charm's schema to "cs" in the DB
+	// In the meantime, we allow fake-juju to have internet access.
+	// XXX (lp:1639276): Remove this special case.
+	utils.OutgoingAccessAllowed = true
+
+	ports := s.api.APIHostPorts()
+	ports[0][0].SpaceName = "dummy-provider-network"
+	err := s.state.SetAPIHostPorts(ports)
+	if err != nil {
+		return err
+	}
+
+	config := map[string]interface{}{"default-series": s.options.Series}
+	err = s.state.UpdateModelConfig(config, nil, nil)
+	if err != nil {
+		return err
+	}
+
+	return nil
+}
+
+// Initialize the controller machine (aka machine 0).
+func (s *FakeJujuService) InitializeController(controller *state.Machine) error {
+	currentVersion := version.Current.String()
+
+	agentVersion, err := semversion.ParseBinary(currentVersion + "-xenial-amd64")
+	if err != nil {
+		return err
+	}
+
+	controller.SetAgentVersion(agentVersion)
+
+	address := network.NewScopedAddress("127.0.0.1", network.ScopeCloudLocal)
+	return controller.SetProviderAddresses(address)
+}
+
+func (s *FakeJujuService) NewInstanceId() instance.Id {
+	s.instanceCount += 1
+	return instance.Id(fmt.Sprintf("id-%d", s.instanceCount))
+}

=== added file 'common/service/service_test.go'
--- common/service/service_test.go	1970-01-01 00:00:00 +0000
+++ common/service/service_test.go	2016-11-30 06:28:02 +0000
@@ -0,0 +1,67 @@
+package service_test
+
+import (
+	"testing"
+
+	gc "gopkg.in/check.v1"
+
+	"github.com/juju/juju/agent"
+	"github.com/juju/juju/state"
+	"github.com/juju/juju/testing/factory"
+	"github.com/juju/juju/version"
+	"github.com/juju/utils"
+
+	coretesting "github.com/juju/juju/juju/testing"
+	jujutesting "github.com/juju/juju/testing"
+
+	"../service"
+)
+
+type FakeJujuServiceSuite struct {
+	coretesting.JujuConnSuite
+	service *service.FakeJujuService
+}
+
+func (s *FakeJujuServiceSuite) SetUpTest(c *gc.C) {
+	s.JujuConnSuite.SetUpTest(c)
+
+	options := &service.FakeJujuOptions{}
+	s.service = service.NewFakeJujuService(s.State, s.APIState, options)
+}
+
+// The Initialize() method performs various initialization tasks.
+func (s *FakeJujuServiceSuite) TestInitialize(c *gc.C) {
+	err := s.service.Initialize()
+	c.Assert(err, gc.IsNil)
+	c.Assert(utils.OutgoingAccessAllowed, gc.Equals, true)
+
+	ports, err := s.State.APIHostPorts()
+	c.Assert(err, gc.IsNil)
+	c.Assert(string(ports[0][0].SpaceName), gc.Equals, "dummy-provider-network")
+}
+
+// The InitializeController() method configures the controller machine.
+func (s *FakeJujuServiceSuite) TestInitializeController(c *gc.C) {
+	controller := s.Factory.MakeMachine(c, &factory.MachineParams{
+		InstanceId: s.service.NewInstanceId(),
+		Nonce:      agent.BootstrapNonce,
+		Jobs:       []state.MachineJob{state.JobManageModel, state.JobHostUnits},
+		Series:     "xenial",
+	})
+	err := s.service.InitializeController(controller)
+	c.Assert(err, gc.IsNil)
+
+	tools, err := controller.AgentTools()
+	c.Assert(err, gc.IsNil)
+	c.Assert(
+		tools.Version.String(),
+		gc.Equals,
+		version.Current.String()+"-xenial-amd64")
+}
+
+var _ = gc.Suite(&FakeJujuServiceSuite{})
+
+// Hook up gocheck into the "go test" runner.
+func Test(t *testing.T) {
+	jujutesting.MgoTestPackage(t)
+}

=== added file 'common/service/suite.go'
--- common/service/suite.go	1970-01-01 00:00:00 +0000
+++ common/service/suite.go	2016-11-30 06:28:02 +0000
@@ -0,0 +1,46 @@
+package service
+
+import (
+	gc "gopkg.in/check.v1"
+	corecharm "gopkg.in/juju/charmrepo.v2-unstable"
+
+	"github.com/juju/juju/agent"
+	"github.com/juju/juju/juju/testing"
+	"github.com/juju/juju/state"
+	"github.com/juju/juju/testing/factory"
+)
+
+// Wrapper to setup and run the core FakeJujuService.
+//
+// It's implemented as a gocheck test suite because that's the easiest way
+// to re-use all the code that sets up the dummy provider. Ideally such
+// logic should be factored out from testing-related tooling and be made
+// standalone.
+type FakeJujuSuite struct {
+	testing.JujuConnSuite
+
+	options *FakeJujuOptions
+	service *FakeJujuService
+}
+
+func (s *FakeJujuSuite) SetUpTest(c *gc.C) {
+	s.JujuConnSuite.SetUpTest(c)
+
+	s.PatchValue(&corecharm.CacheDir, c.MkDir())
+
+	s.service = NewFakeJujuService(s.State, s.APIState, s.options)
+	err := s.service.Initialize()
+	c.Assert(err, gc.IsNil)
+
+	controller := s.Factory.MakeMachine(c, &factory.MachineParams{
+		InstanceId: s.service.NewInstanceId(),
+		Nonce:      agent.BootstrapNonce,
+		Jobs:       []state.MachineJob{state.JobManageModel, state.JobHostUnits},
+		Series:     s.options.Series,
+	})
+	err = s.service.InitializeController(controller)
+	c.Assert(err, gc.IsNil)
+}
+
+func (s *FakeJujuSuite) TestStart(c *gc.C) {
+}