Skip to content
This repository has been archived by the owner on Nov 8, 2022. It is now read-only.

Commit

Permalink
Start of test refactoring:
Browse files Browse the repository at this point in the history
Tagging existing tests as legacy tests, adding a canonical example of a
small test, changing the test matrix, scripts and Makefile to support
the new test types, and modifying the documentation to reflect the
changes made
  • Loading branch information
Tom McSweeney committed May 20, 2016
1 parent 9937476 commit fbc056c
Show file tree
Hide file tree
Showing 69 changed files with 518 additions and 61 deletions.
12 changes: 7 additions & 5 deletions .travis.yml
Original file line number Diff line number Diff line change
@@ -1,10 +1,8 @@
sudo: false
language: go
matrix:
include:
- go: 1.5.4
env: GO15VENDOREXPERIMENT=1
- go: 1.6.2
go:
- 1.5.4
- 1.6.2
before_install:
- bash scripts/gitcookie.sh
- go get github.com/tools/godep
Expand All @@ -18,6 +16,10 @@ env:
global:
- SNAP_SOURCE=/home/travis/gopath/src/github.com/intelsdi-x/snap
- SNAP_PATH=/home/travis/gopath/src/github.com/intelsdi-x/snap/build
- GO15VENDOREXPERIMENT=1
matrix:
- SNAP_TEST_TYPE=legacy
- SNAP_TEST_TYPE=small
install:
- export TMPDIR=$HOME/tmp
- mkdir -p $TMPDIR
Expand Down
12 changes: 11 additions & 1 deletion CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ Contribution of code to snap is a snap (pun intended):
- Fork the project to your own repository
- Create a topic branch from where you want to base your work (usually master)
- Make commit(s) (following commit guidelines below)
- Add any needed test coverage
- Add tests to cover contributed code (if necessary)
- Push your commit(s) to your repository
- Open a pull request against the original repo and follow the pull request guidelines below

Expand All @@ -25,6 +25,16 @@ commit is addressing an open issue, please start the commit message with "Fix: #
"Feature: #XXX". This will help make the generated changelog for each release easy to read
with what commits were fixes and what commits were features.

### Testing Guidelines

For any pull request submitted, the maintainers of Snap require `small` tests that cover the code being modified and/or features being added; `medium` and `large` tests are also welcome but are not required. This breakdown of tests into `small`, `medium`, and `large` is a new taxonomy adopted by the Snap team in May, 2016. These three test types can be best described as follows:
* **Small** tests are written to exercise behavior within a single function or module. While of you might think of these as *unit* tests, a more generic term seems appropriate to avoid any confusion. In general, there is no reliance in a *small* test on any external systems (databases, web servers, etc.), and responses expected from such external services (if any) will be *mocked* or *faked*. When we say reliance on “external systems” we are including reliance on access to the network, the filesystem, external systems (eg. databases), system properties, multiple threads of execution, or the use of sleep statements as part of the test. These tests should be the easiest to automate and the fastest to run (returning a result in a minute or less, with most returning a result in a few seconds or less). These tests will be run automatically on any pull requests received from a contributor, and all *small* tests must pass before a pull request will be reviewed.
* **Medium** tests involve two or more features and test the interaction between those features. For those with previous testing experience, you might think of these as *integration* tests, but because there are a large number of other types of tests that fall into this category a more generic term is needed. The question being answered by these tests is whether or not the interactions between a feature and it’s nearest neighbors interoperate the way that they are expected to. *Medium* tests can rely on access to local services (a local database, for example), the local filesystem, multiple threads of execution, sleep statements and even access to the (local) network. However, reliance on access to external systems and services (systems and services not available on the localhost) in *medium* tests is discouraged. In general, we should expect that these tests return a result in 5 minutes or less, although some *medium* tests may return a result in much less time than that (depending on local system load). These tests can typically be automated and the set of *medium* tests will be run against any builds prior to their release.
* **Large** tests represent typical user scenarios and might be what some of you would think of as *functional* tests. However, as was the case with the previous two categories, we felt that the more generic term used by the Google team seemed to be appropriate here. For these tests, reliance on access to the network, local services (like databases), the filesystem, external systems, multiple threads of execution, system properties, and the use of sleep statements within the tests are all supported. Some of these tests might be run manually as part of the release process, but every effort is made to ensure that even these *large* tests can be automated (where possible). The response times for testing of some of these user scenarios could be 15 minutes or more (eg. it may take some time to bring the system up to an equilibrium state when load testing), so there are situations where these *large* tests will have to be triggered manually even if the test itself is run as an automated test.

This taxonomy is the same taxonomy used by the Google Test team and was described in a posting to the Google Testing Blog that can be found [here](http://googletesting.blogspot.com/2010/12/test-sizes.html).


### Pull Request Guidelines

Pull requests can contain a single commit or multiple commits. The most important part is that _**a single commit maps to a single fix**_. Here are a few scenarios:
Expand Down
10 changes: 9 additions & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,15 @@ default:
deps:
bash -c "./scripts/deps.sh"
test:
export SNAP_PATH=`pwd`/build; bash -c "./scripts/test.sh"
export SNAP_PATH=`pwd`/build; bash -c "./scripts/test.sh $(SNAP_TEST_TYPE)"
test-legacy:
export SNAP_PATH=`pwd`/build; bash -c "./scripts/test.sh legacy"
test-small:
export SNAP_PATH=`pwd`/build; bash -c "./scripts/test.sh small"
test-medium:
export SNAP_PATH=`pwd`/build; bash -c "./scripts/test.sh medium"
test-large:
export SNAP_PATH=`pwd`/build; bash -c "./scripts/test.sh large"
check:
$(MAKE) test
all:
Expand Down
2 changes: 2 additions & 0 deletions control/available_plugin_test.go
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
// +build legacy

/*
http://www.apache.org/licenses/LICENSE-2.0.txt
Expand Down
2 changes: 2 additions & 0 deletions control/config_test.go
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
// +build legacy

/*
http://www.apache.org/licenses/LICENSE-2.0.txt
Expand Down
2 changes: 2 additions & 0 deletions control/control_test.go
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
// +build legacy

/*
http://www.apache.org/licenses/LICENSE-2.0.txt
Expand Down
2 changes: 2 additions & 0 deletions control/metrics_test.go
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
// +build legacy

/*
http://www.apache.org/licenses/LICENSE-2.0.txt
Expand Down
2 changes: 2 additions & 0 deletions control/monitor_test.go
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
// +build legacy

/*
http://www.apache.org/licenses/LICENSE-2.0.txt
Expand Down
2 changes: 2 additions & 0 deletions control/mttrie_test.go
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
// +build legacy

/*
http://www.apache.org/licenses/LICENSE-2.0.txt
Expand Down
2 changes: 2 additions & 0 deletions control/plugin/client/httpjsonrpc_test.go
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
// +build legacy

/*
http://www.apache.org/licenses/LICENSE-2.0.txt
Expand Down
2 changes: 2 additions & 0 deletions control/plugin/collector_proxy_test.go
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
// +build legacy

/*
http://www.apache.org/licenses/LICENSE-2.0.txt
Expand Down
2 changes: 2 additions & 0 deletions control/plugin/collector_test.go
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
// +build legacy

/*
http://www.apache.org/licenses/LICENSE-2.0.txt
Expand Down
2 changes: 2 additions & 0 deletions control/plugin/cpolicy/bool_test.go
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
// +build legacy

/*
http://www.apache.org/licenses/LICENSE-2.0.txt
Expand Down
2 changes: 2 additions & 0 deletions control/plugin/cpolicy/float_test.go
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
// +build legacy

/*
http://www.apache.org/licenses/LICENSE-2.0.txt
Expand Down
2 changes: 2 additions & 0 deletions control/plugin/cpolicy/integer_test.go
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
// +build legacy

/*
http://www.apache.org/licenses/LICENSE-2.0.txt
Expand Down
2 changes: 2 additions & 0 deletions control/plugin/cpolicy/node_test.go
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
// +build legacy

/*
http://www.apache.org/licenses/LICENSE-2.0.txt
Expand Down
2 changes: 2 additions & 0 deletions control/plugin/cpolicy/string_test.go
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
// +build legacy

/*
http://www.apache.org/licenses/LICENSE-2.0.txt
Expand Down
2 changes: 2 additions & 0 deletions control/plugin/cpolicy/tree_test.go
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
// +build legacy

/*
http://www.apache.org/licenses/LICENSE-2.0.txt
Expand Down
2 changes: 2 additions & 0 deletions control/plugin/encrypter/encrypter_test.go
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
// +build legacy

/*
http://www.apache.org/licenses/LICENSE-2.0.txt
Expand Down
2 changes: 2 additions & 0 deletions control/plugin/execution_test.go
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
// +build legacy

/*
http://www.apache.org/licenses/LICENSE-2.0.txt
Expand Down
2 changes: 2 additions & 0 deletions control/plugin/metric_test.go
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
// +build legacy

/*
http://www.apache.org/licenses/LICENSE-2.0.txt
Expand Down
2 changes: 2 additions & 0 deletions control/plugin/plugin_test.go
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
// +build legacy

/*
http://www.apache.org/licenses/LICENSE-2.0.txt
Expand Down
2 changes: 2 additions & 0 deletions control/plugin/processor_test.go
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
// +build legacy

/*
http://www.apache.org/licenses/LICENSE-2.0.txt
Expand Down
2 changes: 2 additions & 0 deletions control/plugin/publisher_test.go
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
// +build legacy

/*
http://www.apache.org/licenses/LICENSE-2.0.txt
Expand Down
2 changes: 2 additions & 0 deletions control/plugin/session_test.go
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
// +build legacy

/*
http://www.apache.org/licenses/LICENSE-2.0.txt
Expand Down
2 changes: 2 additions & 0 deletions control/plugin_manager_test.go
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
// +build legacy

/*
http://www.apache.org/licenses/LICENSE-2.0.txt
Expand Down
2 changes: 2 additions & 0 deletions control/runner_test.go
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
// +build legacy

/*
http://www.apache.org/licenses/LICENSE-2.0.txt
Expand Down
2 changes: 2 additions & 0 deletions control/strategy/cache_test.go
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
// +build legacy

/*
http://www.apache.org/licenses/LICENSE-2.0.txt
Expand Down
2 changes: 2 additions & 0 deletions control/strategy/sticky_test.go
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
// +build legacy

/*
http://www.apache.org/licenses/LICENSE-2.0.txt
Expand Down
2 changes: 2 additions & 0 deletions core/cdata/node_test.go
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
// +build legacy

/*
http://www.apache.org/licenses/LICENSE-2.0.txt
Expand Down
2 changes: 2 additions & 0 deletions core/cdata/tree_test.go
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
// +build legacy

/*
http://www.apache.org/licenses/LICENSE-2.0.txt
Expand Down
2 changes: 2 additions & 0 deletions core/plugin_test.go
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
// +build legacy

/*
http://www.apache.org/licenses/LICENSE-2.0.txt
Expand Down
88 changes: 72 additions & 16 deletions docs/BUILD_AND_TEST.md
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
-->
-->
# Build and Test
This guide gets you started with building and testing Snap. If you have commits you want to contribute, review the [CONTRIBUTING file](../CONTRIBUTING.md) for a shorter list of what we look for and come back here if you need to verify your environment is configured correctly.

Expand All @@ -30,7 +30,7 @@ To build the Snap Framework you'll need:

The instructions below assume that the `GOPATH` environment variable has been set properly. Many of us use [go version manager (gvm)](https://github.com/moovweb/gvm) to easily switch between Go versions.

Now you can install Snap into your `$GOPATH`:
Now you can install Snap into your `$GOPATH`:
```
$ go get github.com/intelsdi-x/snap
$ cd $GOPATH/src/github.com/intelsdi-x/snap
Expand Down Expand Up @@ -68,36 +68,92 @@ To see how to use Snap, look at [Running Snap](../README.md#running-snap), [SNAP
### Creating Tests
Our tests are written using [smartystreets' GoConvey package](https://github.com/smartystreets/goconvey). See https://github.com/smartystreets/goconvey/wiki for an introduction to creating a test using this package.

### Tests in Golang
We follow the Golang methodology of placing tests into files with names that look like `*_test.go`. See [this](https://golang.org/cmd/go/#hdr-Test_packages) section from [go command](https://golang.org/cmd/go/) documentation for more details

### Test Types in Snap
As was mentioned previously, tests in Snap are broken down into `small`, `medium`, and `large` tests. Definitions for these test types can be found in the 'Testing Guidelines' section [CONTRIBUTING.md](../CONTRIBUTING.md) document.

But, how do you actually tag a test as *small*, *medium*, or *large* in Golang? As it turns out, this is a relatively simple process. To tag a given test file by the category of tests that it contains, all we have to do is add a single line to the beginning of the file that provides a *build tag* for that file. For example, a line like this:

// +build small

would identify that file as a file that contains *small* tests, while a line like this as the first line of the file:

// +build medium

would identify that file as a file that contains *medium* tests. Once those build tags are have been added to the test files in the Snap codebase, it is relative simple to run a specific set of tests (by type) by simply adding a `-tags [TAG]` command-line flag to the `go test` command (where the `[TAG]` value is replaced by one of our test types). For example, this command will run all of the tests in the current working directory or any of it’s subdirectories that have been tagged as *small* tests:

$ go test -v -tags=small ./...

It should be noted here that if there are any untagged tests in the directory referenced by the `go test` command, those untagged tests will also be run, regardless of the `-tags [TAG]` option that is passed into the `go test` command. To deal with this issue, all preexisting tests in the Snap framework have been tagged with a *legacy* tag. We have also modified the current test matrix (in the appropriate TravisCI and test shell-script files) to run both the *small* and *legacy* tests until such time that we feel that there is sufficient test coverage by the *small* tests that are currently being developed).

Once the maintainers feel that the *small* tests provide sufficient code coverage, the existing *legacy* tests will be phased out (or used in the construction of a set of *medium* and *large* tests for the Snap CI/CD toolchain). All new tests being added to the Snap framework by contributors should be marked as either `small`, `medium`, or `large` tests, depending on their scope.

### Running Tests
#### In local machine
First you need to run the following to get all test dependencies (this runs ./scripts/test.sh as well):
#### On a local machine
Contributors are assumed to have run the `legacy` and `small` tests that cover their contribution on their local machine prior to submitting the corresponding pull request. There are several ways of accomplishing this depending on the coverage required.

Before any tests can be run, the following command should be executed to pull down all of the test dependencies to the local machine (note; this will also run the `./scripts/test.sh` script, see below for more details):
```
$ cd $GOPATH/src/github.com/intelsdi-x/snap
$ make test
$ SNAP_TEST_TYPE=[SNAP_TEST_TYPE] make test
```
where the string `[SNAP_TEST_TYPE]` that is shown above is replaced with one of our test types (`legacy`, `small`, `medium`, or `large`). Once the dependencies have been pulled down to the local machine, there are a number of ways to run the various tests included in the Snap framework. To run the complete set of `legacy`, `small`, `medium`, or `large` tests in the Snap framework (and stop at the first failure in a directory before continuing on), you can simply use the same shell-script that is run by the `make test` command that was shown above:
```
$ ./scripts/test.sh [SNAP_TEST_TYPE]
```
As was the case in the `make test` command that was shown above, the string `[SNAP_TEST_TYPE]` that is shown in this command should be replaced by the type of tests you wish to run (`legacy`, `small`, `medium`, or `large`). Note, that for convenience we have also added four new targets to the Snap `Makefile` that can be used to run the `legacy`, `small`, `medium`, or `large` tests in a simple `make test-*` command. To run the `small` tests, for example, one could simply run a command that looks something like this:
```
$ make test-small
```
To run the other types of tests in the Snap framework, simply replace the `small` type with one of the other types (`legacy`, `medium`, or `large` in the example `make test-*` command shown above).

To run all tests in order (it will stop at any failure in a directory before continuing on):
If you are interested in running all of the `legacy` tests from the Snap framework *and continuing through to all subdirectories, regardless of any errors that might be encountered*, then you can run a `go test ...` command directly (instead of running a `make test-*` or `scripts/test.sh [SNAP_TEST_TYPE]` command like those shown above). That `go test ...` command would look something like this:
```
$ ./scripts/test.sh
go test -tags=legacy ./...
```
It should be noted here that the `go test ...` command shown above will iterate over all of the subdirectories of the current working directory and run all of the `legacy` tests found. It will not stop if errors are detected during a test run; those errors will simply be reported as part of the output of the `go test ...` command.

To run all tests *and to continue through all directories even with errors*:
To run all of the small tests from the Snap framework, one would simply change the value of the tag passed into the command from `legacy` to `small`:
```
go test -tags=small ./...
```
$ go test ./...
```

Tests can be pruned in go test with the `-run` option followed by the test name (e.g. `TestLoad` from `control_test.go`):
All tests in the Snap framework are tagged as `legacy`, `small`, `medium`, or `large`, so at least one of those tags must be provided to any `go test ...` command that is executed by a contributor.

Individual tests from the framework can also be selected and run by passing a test name into the `go test...` command using the `-run` command-line flag:
```
$ go test ./... -v -run TestLoad
go test -v -tags=legacy -run=[TEST_NAME] ./...
```

Or using GoConvey ([in a browser](https://github.com/smartystreets/goconvey#in-the-browser)):
e.g. to run the `TestLoad` test from `control_test.go`, one would simply run a command that looks something like this:
```
go test -v -tags=legacy -run=TestLoad ./...
```
Finally, coverage reports can be generated that show (on a function-by-function level) how well a given test or set of tests cover the current codebase. For example, the following pair of commands will generate a report showing the coverage of all of the `legacy` tests in the current working directory (as text output to the console):
```
$ go test -coverprofile=/tmp/coverage.out && go tool cover -html=/tmp/coverage.out
go test -tags=small -coverprofile=/tmp/coverage.out . && go tool cover -func=/tmp/coverage.out
```
A similar pair of commands can be used to generate an HTML view showing the same information, but in a browsable view that shows how the matching tests cover the code on a line-by-line basis for each file in the current working directory (the resulting view will open in the default web browser defined on the local machine)
```
go test -tags=small -coverprofile=/tmp/coverage.out . && go tool cover -html=/tmp/coverage.out
```
As was the case with the previous commands, the `-run` command-line flag can be used to further reduce the scope of the generated report to only show the coverage of named test. It should be noted here that these coverage reports can only be generated for code in a single directory. The `-coverprofile` command-line flag cannot be used to generate a coverage report across all subdirectories as was shown in the `go test ...` command shown previously.

### Building Effective Small Tests

Any `small` tests added to the Snap framework must conform to the following constraints:
* They should test the behavior of a single function or method in the framework
* There should no reliance rely on external systems or system-level resources (eg. networks, filesystem, external systems or services, system properties, multiple threads of execution, or the use of sleep statements) as part of these tests; the expected responses of any such external systems or access to system-level resources should be mocked appropriately.
* They should be independent of any other tests in the test framework
* They should return the same result every time they are run, regardless of the environment they are run in (a failure of any of these tests should be indicative of an issue with the code being tested, not the environment that the test is being run in)

When complete, the full set of `small` tests for any given function or method should provide sufficient code coverage to ensure that any changes made to that function or method will not 'break the build'. This will assure the Snap maintainers that any pull requests that are made to modify or add to the framework can be safely merged (provided that there is sufficient code coverage and the associated tests pass).

It should be noted here that the maintainers will refuse to merge any pull requests that trigger a failure of any of the `small` or `legacy` tests that cover the code being modified or added to the framework. As such, we highly recommend that contributors run the tests that cover their contributions locally before submitting their contribution as a pull request. Maintainers may also ask that contributors add tests to their pull requests to ensure adequate code coverage before the they are willing to accept a given pull request, even if all existing tests pass. Our hope is that you, as a contributor, will understand the need for this requirement.

#### In Docker
The Snap Framework supports running tests in an isolated container as opposed to your local host. Run the test script, which calls a `Dockerfile` located at `./scripts/Dockerfile`:
The Snap Framework supports running tests in an isolated container as opposed to your local host. Run the test script, which calls a `Dockerfile` located at `./scripts/Dockerfile`:

```
$ cd $GOPATH/src/github.com/intelsdi-x/snap
Expand Down
2 changes: 2 additions & 0 deletions mgmt/rest/client/client_config_func_test.go
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
// +build legacy

/*
http://www.apache.org/licenses/LICENSE-2.0.txt
Expand Down
Loading

0 comments on commit fbc056c

Please sign in to comment.