github.com/mvdan/u-root-coreutils@v0.0.0-20230122170626-c2eef2898555/CONTRIBUTING.md (about)

     1  # Contributing to u-root
     2  
     3  We need help with this project. Contributions are very welcome. See the [roadmap](roadmap.md), open [issues](https://github.com/u-root/u-root/issues), and join us in [Slack](CONTRIBUTING.md#communication) to talk about your cool ideas for the project.
     4  
     5  ## Code of Conduct
     6  
     7  Conduct collaboration around u-root in accordance to the [Code of
     8  Conduct](https://github.com/u-root/u-root/wiki/Code-of-Conduct).
     9  
    10  ## Communication
    11  
    12  - [Open Source Firmware Slack team](https://osfw.slack.com),
    13    channel `#u-root`, sign up [here](https://slack.osfw.dev/)
    14  - [Join the mailing list](https://groups.google.com/forum/#!forum/u-root)
    15  
    16  ## Bugs
    17  
    18  - Please submit issues to https://github.com/u-root/u-root/issues
    19  
    20  ## Coding Style
    21  
    22  The ``u-root`` project aims to follow the standard formatting recommendations
    23  and language idioms set out in the [Effective Go](https://golang.org/doc/effective_go.html)
    24  guide, for example [formatting](https://golang.org/doc/effective_go.html#formatting)
    25  and [names](https://golang.org/doc/effective_go.html#names).
    26  
    27  `gofmt` and `golint` are law, although this is not automatically enforced
    28  yet and some housecleaning needs done to achieve that.
    29  
    30  We have a few rules not covered by these tools:
    31  
    32  - Standard imports are separated from other imports. Example:
    33      ```
    34      import (
    35        "regexp"
    36        "time"
    37  
    38        dhcp "github.com/krolaw/dhcp4"
    39      )
    40      ```
    41  
    42  ## General Guidelines
    43  
    44  We want to implement some of the common commands that exist in upstream projects and elsewhere, but we don't need to copy broken behavior. CLI compatibility with existing implementations isn't required. We can add missing functionality and remove broken behavior from commands as needed.
    45  
    46  U-root needs to fit onto small flash storage, (eg. 8 or 16MB SPI). Be cognizant of of how your work is increasing u-root's footprint. The current goal is to keep the BB mode `lzma -9` compressed initramfs image under 3MB.
    47  
    48  ## Pull Requests
    49  
    50  We accept GitHub pull requests.
    51  
    52  Fork the project on GitHub, work in your fork and in branches, push
    53  these to your GitHub fork, and when ready, do a GitHub pull requests
    54  against https://github.com/u-root/u-root.
    55  
    56  u-root uses Go modules for its dependency management, but still vendors
    57  dependencies in the repository pending module support in the build system.
    58  Please run `go mod tidy` and `go mod vendor` and commit `go.mod`, `go.sum`, and
    59  `vendor/` changes before opening a pull request.
    60  
    61  Organize your changes in small and meaningful commits which are easy to review.
    62  Every commit in your pull request needs to be able to build and pass the CI tests.
    63  
    64  If the pull request closes an issue please note it as: `"Fixes #NNN"`.
    65  
    66  ### Patch Format
    67  
    68  Well formatted patches aide code review pre-merge and code archaeology in
    69  the future.  The abstract form should be:
    70  ```
    71  <component>: Change summary
    72  
    73  More detailed explanation of your changes: Why and how.
    74  Wrap it to 72 characters.
    75  See [here] (http://chris.beams.io/posts/git-commit/)
    76  for some more good advices.
    77  
    78  Signed-off-by: <contributor@foo.com>
    79  ```
    80  
    81  An example from this repo:
    82  ```
    83  tcz: quiet it down
    84  
    85  It had a spurious print that was both annoying and making
    86  boot just a tad slower.
    87  
    88  Signed-off-by: Ronald G. Minnich <rminnich@gmail.com>
    89  ```
    90  
    91  ### Developer Sign-Off
    92  
    93  For purposes of tracking code-origination, we follow a simple sign-off
    94  process.  If you can attest to the [Developer Certificate of
    95  Origin](https://developercertificate.org/) then you append in each git
    96  commit text a line such as:
    97  ```
    98  Signed-off-by: Your Name <username@youremail.com>
    99  ```
   100  
   101  ### Incorporation of Feedback
   102  To not break the conversation history inside the PR avoid force pushes. Instead, push further _'fix up commits'_ to resolve annotations.
   103  Once review is done, do a local rebase to clean up the _'fix up commits'_ and come back to a clean commit history and do a single fore push to the PR.
   104  
   105  ## Unit Testing Guidelines
   106  
   107  ### Unit Test Checks
   108  
   109  * The [testify](https://github.com/stretchr/testify) package should not be used.
   110  * The [cmp](https://pkg.go.dev/github.com/google/go-cmp/cmp) package is allowed.
   111  * Unit tests in Go should follow the guidelines in this tutorial: https://go.dev/doc/tutorial/add-a-test
   112    * In particular, the test error should be in the form `Function(...) = ...; want ...`.
   113  
   114  For example:
   115  
   116  ```
   117  if msg != "" || err == nil {
   118  	t.Fatalf(`Hello("") = %q, %v, want "", error`, msg, err)
   119  }
   120  ```
   121  
   122  * Tests should do all filesystem changes under a temporary directory, either
   123    created with `ioutil.TempDir` or `testing.T.TempDir`.
   124  
   125  ### Mocking Dependencies
   126  
   127  * Special mocking packages should not be used.
   128  * Prefer to test the real thing where possible.
   129  * Mocking is sometimes necessary. For example:
   130     * Operations as root.
   131     * Interacting with special hardware (ex: USB, SPI, PCIe)
   132     * Modifying machine state (ex: reboot, kexec)
   133     * Tests which would otherwise be flaky (ex: `time.Sleep`, networking)
   134  * Consider writing an integration test if the program can not be easily run
   135    directly.
   136     * Integration tests let you run the command under qemu, which lets you test
   137       operations with, e.g., virtual hardware.
   138    * `pkg/mount` contains an example of an integration test run under QEMU.
   139  * Prefer to mock using existing interfaces. For example: `io.Reader`, `fs.FS`
   140  * Avoid mocking using global state. Instead, consider using one of the
   141    following "dependency injection" idioms:
   142  
   143  1. Mocking functions:
   144  
   145  ```
   146  // The exported function has a meaningful API.
   147  func SetMemAddr(addr, val uint) error {
   148  	return setMemAddr(addr, val, "/dev/mem")
   149  }
   150  
   151  // The internal function is called from the unit test. The test can set a
   152  // different `file` argument.
   153  func setMemAddr(addr, val uint, file string) error {
   154  	f, err := os.Open(file)
   155  	if err != nil {
   156  		return err
   157  	}
   158  	...
   159  }
   160  ```
   161  
   162  2. Mocking objects:
   163  
   164  ```
   165  // SPI interface for the underlying calls to the SPI driver.
   166  type SPI interface {
   167  	Transfer(transfers []spidev.Transfer) error
   168  }
   169  
   170  // Flash provides operations for SPI flash chips.
   171  type Flash struct {
   172  	// spi is the underlying SPI device.
   173  	spi SPI
   174  
   175    // other fields ...
   176  }
   177  
   178  // New creates a new flash device from a SPI interface.
   179  func New(spi SPI) (*Flash, error) {
   180  	f := &Flash{
   181  		spi: spi,
   182  	}
   183  
   184    // initialize other fields ...
   185  
   186    return f
   187  }
   188  ```
   189  
   190  In the above example, the `flash.New` function takes a `SPI` device which can
   191  be mocked out as follows:
   192  
   193  ```
   194  f, err := flash.New(spimock.New())
   195  ...
   196  ```
   197  
   198  The `spimock.New()` function returns an implementation of SPI which mocks the
   199  underlying SPI driver. The `Flash` object can be tested without hardware
   200  dependencies.
   201  
   202  ### VM Tests using QEMU
   203  For code reading or manipulating hardware, it can be reasonable not to mock out syscalls etc. but to run tests in a virtual environment.
   204  In case you need to test against certain hardware, you can use a QEMU environment via `pkg/vmtest`. In your package, put the setup and corresponding tests in `vm_test.go`. 
   205  
   206  **IMPORTANT Notes**
   207  * Add `!race` build tag to your `vm_test.go`
   208  * Setup QEMU inside a usual test function
   209  * Make sure the tests assuming this setup are skipped in non-VM test run
   210  * Add your package to `blocklist` in `integration/gotests/gotest_test.go` making sure the test doesn't run in the project wide integration tests without the proper QEMU setup.
   211  
   212  See below or `pkg/mount/block` for examples.
   213  ```
   214  //go:build !race
   215  // +build !race
   216  
   217  package foo
   218  
   219  import (
   220  	"github.com/u-root/u-root/pkg/qemu"
   221  	"github.com/u-root/u-root/pkg/testutil"
   222  	"github.com/u-root/u-root/pkg/vmtest"
   223  )
   224  
   225  // VM setup:
   226  //
   227  //  Describe your setup
   228  
   229  func TestVM(t *testing.T) {
   230  	o := &vmtest.Options{
   231  		QEMUOpts: qemu.Options{
   232  			Devices: []qemu.Device{
   233  				// Configure QEMU here
   234  				qemu.ArbitraryArgs{...},
   235  			},
   236  		},
   237  	}
   238  	vmtest.GolangTest(t, []string{"github.com/u-root/u-root/pkg/foo"}, o)
   239  }
   240  
   241  func TestFooRunInQEMU1(t *testing.T) {
   242    if os.Getuid != 0 {
   243      t.Skip("Skipping since we are not root")
   244    }
   245    // ...
   246  }
   247  
   248  func TestFooRunInQEMU2(t *testing.T) {
   249    if os.Getuid != 0 {
   250      t.Skip("Skipping since we are not root")
   251    }
   252    // ...
   253  }
   254  
   255  // ...
   256  
   257  ```
   258  
   259  ### Package main
   260  
   261  The main function often includes things difficult to test. For example:
   262  
   263  1. Process-ending functions such as `log.Fatal` and `os.Exit`. These functions
   264     also kill the unit test process.
   265  2. Accessing global state such as `os.Args`, `os.Stdin` and `os.Stdout`. It is
   266     hard to mock out global state cleanly and safely.
   267     
   268  **Do not use `pkg/testutil` it is deprecated. Instead go with the following:**
   269  
   270  The guideline for testing is to factor out everything "difficult" into a
   271  two-line `main` function which remain untested. For example:
   272  
   273  ```
   274  func run(args []string, stdin io.Reader, stdout io.Writer) error {
   275  	...
   276  }
   277  
   278  func main() {
   279  	if err := run(os.Args[1:], os.Stdin, os.Stdout); err != nil {
   280  		log.Fatalf("Error: %v", err)
   281  	}
   282  }
   283  ```
   284  
   285  ### Integration Tests
   286  To test integration with other code/packages without mocking, write integration test in a file `integration_test.go`.
   287  
   288  ## Code Reviews
   289  
   290  Look at the area of code you're modifying, its history, and consider
   291  tagging some of the [maintainers](https://u-root.tk/#contributors) when doing a
   292  pull request in order to instigate some code review.
   293  
   294  ## Quality Controls
   295  
   296  [CircleCI](https://circleci.com/gh/u-root/u-root) is used to test and build commits in a pull request.
   297  
   298  See [.circleci/config.yml](.circleci/config.yml) for the CI commands run.
   299  You can use [CircleCI's CLI tool](https://circleci.com/docs/2.0/local-cli/#run-a-job-in-a-container-on-your-machine) to run individual jobs from `.circlecl/config.yml` via Docker, eg. `circleci build --job test`.