From 1999d685910d14dc2272eb3e4d8eab7091704893 Mon Sep 17 00:00:00 2001 From: unclegedd Date: Thu, 9 May 2024 11:54:10 -0500 Subject: [PATCH 1/2] chore: update contributing doc --- CONTRIBUTING.md | 27 ++++++++++++++++++++++++--- src/pkg/bundle/common_test.go | 14 ++++++++++---- src/pkg/bundle/deploy_test.go | 6 +----- 3 files changed, 35 insertions(+), 12 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index d0f69945..ce436a93 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -9,7 +9,7 @@ Fundamentally, software engineering is a communication problem; we write code fo - **Write tests that give confidence**: Unless there is a technical blocker, every new feature and bug fix should be tested in the project's automated test suite. Although many of our tests are E2E, unit and integration-style tests are also welcomed. Note that unit tests can live in a `*_test.go` file alongside the source code, and E2E tests live in `src/test/e2e` -- **Prefer readability over being clever**: "I'm old! I don't like complicated things" - @mikevanhemert. We have a strong preference for code readabilty in UDS CLI. Specifically, this means things like: naming variables appropriately, keeping functions to a reasonable size and avoiding complicated solutions when simple ones exist. +- **Prefer readability over being clever**: We have a strong preference for code readabilty in UDS CLI. Specifically, this means things like: naming variables appropriately, keeping functions to a reasonable size and avoiding complicated solutions when simple ones exist. - **User experience is paramount**: UDS CLI doesn't have a pretty UI (yet), but the core user-centered design principles that apply when building a frontend also apply to this CLI tool. First and foremost, features in UDS CLI should enhance workflows and make life easier for end users; if a feature doesn't accomplish this, it will be dropped. @@ -17,8 +17,7 @@ Fundamentally, software engineering is a communication problem; we write code fo ### Pre-Commit Hooks and Linting In this repo you can optionally use [pre-commit](https://pre-commit.com/) hooks for automated validation and linting, but if not CI will run these checks for you. - -## Continuous Delivery +### Continuous Delivery Continuous Delivery is core to our development philosophy. Check out [https://minimumcd.org](https://minimumcd.org/) for a good baseline agreement on what that means. Specifically: @@ -27,3 +26,25 @@ Specifically: - We don't merge code into `main` that isn't releasable - We perform automated testing on all changes before they get merged to `main` - We create immutable release artifacts + +## How to Contribute +Please ensure there is a Gitub issue for your proposed change, this helps the UDS CLI team to understand the context of the change and to track the progress of the work. If there isn't an issue for your change, please create one before starting work. The recommended workflow for contributing is as follows: + +1. **Fork this repo** and clone it locally +2. **Create a branch** for your changes +3. **Create and [test](#testing)** your changes +4. **Push your branch** to your fork +5. **Open a PR** against the `main` branch of this repo + +### Testing + +We strive to test all changes made to UDS CLI. If you're adding a new feature or fixing a bug, please add tests to cover the new functionality. Unit tests and E2E tests are both welcome, but we leave it up to the contributor to decide which is most appropriate for the change. Below are some guidelines for testing: + +#### Unit Tests +Unit tests reside alongside the source code in a `*_test.go` file. These tests should be used to test individual functions or methods in isolation. Unit tests should be fast and focused on a single piece of functionality. + +#### E2E Tests +E2E tests reside in the `src/test/e2e` directory. They use bundles located in the `src/test/e2e/bundles` which contain Zarf packages from the `src/test/e2e/packages` directory. Feel free to add new bundles and packages where appropriate. It's encouraged to write comments/metadata in any new bundles or packages to explain what they are testing. Note that to run E2E tests, you'll need build UDS CLI locally, and re-build any time you make a change to the source code; this is because the binary in the `build` directory is used to drive the tests. + +#### Assertions +We prefer to use Testify's [require](https://github.com/stretchr/testify/tree/master/require) package for assertions in tests. This package provides a rich set of assertion functions that make tests more readable and easier to debug. See other tests in this repo for examples. diff --git a/src/pkg/bundle/common_test.go b/src/pkg/bundle/common_test.go index daa5eadd..d7ec1333 100644 --- a/src/pkg/bundle/common_test.go +++ b/src/pkg/bundle/common_test.go @@ -65,9 +65,12 @@ func Test_validateBundleVars(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - if err := validateBundleVars(tt.args.packages); (err != nil) != tt.wantErr { - t.Errorf("validateBundleVars() error = %v, wantErr %v", err, tt.wantErr) + err := validateBundleVars(tt.args.packages) + if tt.wantErr { + require.Error(t, err) + return } + require.NoError(t, err) }) } } @@ -140,9 +143,12 @@ func Test_validateOverrides(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - if err := validateOverrides(tt.args.bundlePackage, tt.args.zarfPackage); (err != nil) != tt.wantErr { - t.Errorf("validateOverrides() error = %v, wantErr %v", err, tt.wantErr) + err := validateOverrides(tt.args.bundlePackage, tt.args.zarfPackage) + if tt.wantErr { + require.Error(t, err) + return } + require.NoError(t, err) }) } } diff --git a/src/pkg/bundle/deploy_test.go b/src/pkg/bundle/deploy_test.go index 38694851..3cf07c4a 100644 --- a/src/pkg/bundle/deploy_test.go +++ b/src/pkg/bundle/deploy_test.go @@ -2,7 +2,6 @@ package bundle import ( "os" - "reflect" "testing" "github.com/defenseunicorns/uds-cli/src/types" @@ -234,10 +233,7 @@ func TestLoadVariablesPrecedence(t *testing.T) { os.Setenv("UDS_FOO", "set using env var") } actualPkgVars := tc.bundle.loadVariables(tc.pkg, tc.bundleExportVars) - - if !reflect.DeepEqual(actualPkgVars, tc.expectedPkgVars) { - t.Errorf("Test case %s failed. Expected %v, got %v", tc.name, tc.expectedPkgVars, actualPkgVars) - } + require.Equal(t, tc.expectedPkgVars, actualPkgVars) }) } } From a8f84675e81389d71bef16a1412fc06d292bcb0e Mon Sep 17 00:00:00 2001 From: unclegedd Date: Thu, 9 May 2024 13:22:05 -0500 Subject: [PATCH 2/2] more details --- CONTRIBUTING.md | 20 +++++++++++++++----- 1 file changed, 15 insertions(+), 5 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index ce436a93..0dc58758 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -31,10 +31,14 @@ Specifically: Please ensure there is a Gitub issue for your proposed change, this helps the UDS CLI team to understand the context of the change and to track the progress of the work. If there isn't an issue for your change, please create one before starting work. The recommended workflow for contributing is as follows: 1. **Fork this repo** and clone it locally -2. **Create a branch** for your changes -3. **Create and [test](#testing)** your changes -4. **Push your branch** to your fork -5. **Open a PR** against the `main` branch of this repo +1. **Create a branch** for your changes +1. **Create, [test](#testing)** your changes +1. **Add docs** where appropriate +1. **Push your branch** to your fork +1. **Open a PR** against the `main` branch of this repo + +### Building the app +Today, we use `make` to build UDS CLI. To build the app, check out the [Makefile](Makefile) and find the appropriate build target for your system, and run it from the root of the repo (ex. `make build-cli-mac-apple`). This will create a binary in the `build` directory that you can use to test your changes (note that this binary is automatically used when running the E2E tests). ### Testing @@ -44,7 +48,13 @@ We strive to test all changes made to UDS CLI. If you're adding a new feature or Unit tests reside alongside the source code in a `*_test.go` file. These tests should be used to test individual functions or methods in isolation. Unit tests should be fast and focused on a single piece of functionality. #### E2E Tests -E2E tests reside in the `src/test/e2e` directory. They use bundles located in the `src/test/e2e/bundles` which contain Zarf packages from the `src/test/e2e/packages` directory. Feel free to add new bundles and packages where appropriate. It's encouraged to write comments/metadata in any new bundles or packages to explain what they are testing. Note that to run E2E tests, you'll need build UDS CLI locally, and re-build any time you make a change to the source code; this is because the binary in the `build` directory is used to drive the tests. +E2E tests reside in the `src/test/e2e` directory. They use bundles located in the `src/test/e2e/bundles` which contain Zarf packages from the `src/test/e2e/packages` directory. Feel free to add new bundles and packages where appropriate. It's encouraged to write comments/metadata in any new bundles or packages to explain what they are testing. #### Assertions We prefer to use Testify's [require](https://github.com/stretchr/testify/tree/master/require) package for assertions in tests. This package provides a rich set of assertion functions that make tests more readable and easier to debug. See other tests in this repo for examples. + +#### Running Tests +- **Unit Tests**: To run unit tests, run `make test-unit` from the root of the repo. This will run all unit tests in the `src` directory. + + +- **E2E Tests**: To run E2E tests, you'll need build UDS CLI locally, and re-build any time you make a change to the source code; this is because the binary in the `build` directory is used to drive the tests. To run the entire suite of E2E tests locally, run `make test-e2e-no-ghcr-write` (note that this intentionally skips the tests that involve writing to GHCR).