Skip to content

Commit

Permalink
[Cleanup] Initial documentation improvements and code cleanup (#8)
Browse files Browse the repository at this point in the history
## Overview

This PR addresses the lack of documentation in the repo by adding some
details to the `README.md` file and adds some more godoc comments
throughout the codebase.

The workflow files have been updated to a use go version 1.18 and have
been updated to fully test the repo and produce code coverage reports
and check against linters.

The legacy fuzzing has been removed and instead now uses go native
fuzzing, the fuzzing tests remain the same and can be improved upon in a
separate PR. The oss-fuzz build script has been removed until this repo
is on oss-fuzz a new script will not be made.

Additionally linter errors have been addressed.

## Issue

Fixes #7 

Additional testing enhancements will be covered in a separate PR

## Checklist

- [x] Update any relevant README(s)
- [x] Add or update any relevant or supporting
[mermaid](https://mermaid-js.github.io/mermaid/) diagrams
- [ ] Task specific tests or benchmarks: go test ...
- [x] New tests or benchmarks: go test ...
- [x] All tests: go test -v

---------

Co-authored-by: Daniel Olshansky <[email protected]>
  • Loading branch information
h5law and Olshansk authored Jun 1, 2023
1 parent 5c4037e commit cc555d9
Show file tree
Hide file tree
Showing 18 changed files with 598 additions and 251 deletions.
65 changes: 65 additions & 0 deletions .github/PULL_REQUEST_TEMPLATE/pull_request_template.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
<!-- REMOVE this comment block after following the instructions
1. Make the title of the PR is descriptive and follows this format: `[PR TYPE] <DESCRIPTION>`
2. Update the _Assigness_, _Labels_, _Projects_, _Milestone_ before submitting the PR for review.
3. Add label(s) for the purpose (e.g. `persistence`) and, if applicable, priority (e.g. `low`) labels as well.
4. See our custom action driven labels if you need to trigger a build or interact with an LLM - https://github.com/pokt-network/pocket/blob/main/docs/development/README.md#github-labels
-->

## Description

<!-- REMOVE this comment block after following the instructions
1. Add a summary of the change including: motivation, reasons, context, dependencies, etc...
2. If applicable, specify the key files that should be looked at.
3. If you leave the `reviewpad:summary` block below, it'll autopopulate an AI generated summary. Alternatively, you can leave a `/reviewpad summarize` comment to trigger it manually.
-->
reviewpad:summary

## Issue

Fixes #<issue_number>

## Type of change

Please mark the relevant option(s):

- [ ] New feature, functionality or library
- [ ] Bug fix
- [ ] Code health or cleanup
- [ ] Major breaking change
- [ ] Documentation
- [ ] Other <!-- add details here if it a different type of change -->

## List of changes

<!-- REMOVE this comment block after following the instructions
List out all the changes made
-->

- Change #1
- Change #2
- ...

## Testing

- [ ] **Task specific tests or benchmarks**: `go test ...`
- [ ] **New tests or benchmarks**: `go test ...`
- [ ] **All tests**: `go test -v`

<!-- REMOVE this comment block after following the instructions
If you added additional tests or infrastructure, describe it here.
Bonus points for images and videos or gifs.
-->

## Required Checklist

- [ ] I have performed a self-review of my own code
- [ ] I have commented my code, particularly in hard-to-understand areas
- [ ] I have added, or updated, [`godoc` format comments](https://go.dev/blog/godoc) on touched members (see: [tip.golang.org/doc/comment](https://tip.golang.org/doc/comment))
- [ ] I have tested my changes using the available tooling
- [ ] I have updated the corresponding CHANGELOG

### If Applicable Checklist

- [ ] Update any relevant README(s)
- [ ] Add or update any relevant or supporting [mermaid](https://mermaid-js.github.io/mermaid/) diagrams
- [ ] I have added tests that prove my fix is effective or that my feature works
27 changes: 0 additions & 27 deletions .github/workflows/fuzz_build.yml

This file was deleted.

106 changes: 73 additions & 33 deletions .github/workflows/test.yml
Original file line number Diff line number Diff line change
@@ -1,62 +1,102 @@
name: Tests
name: Test & Build

on:
pull_request:
push:
branches:
- master
- main
- release/**

env:
# Even though we can test against multiple versions, this one is considered a target version.
TARGET_GOLANG_VERSION: "1.18"

jobs:
build:
tests:
runs-on: ubuntu-latest
strategy:
fail-fast: false
matrix:
goarch: ["arm64", "amd64"]
timeout-minutes: 5
go: ["1.18"]
name: Go ${{ matrix.go }} test
steps:
- uses: actions/setup-go@v2
- uses: actions/checkout@v3
- name: Setup go
uses: actions/setup-go@v3
with:
go-version: "1.15"
- uses: actions/checkout@v2
- uses: technote-space/get-diff-action@v4
go-version: ${{ matrix.go }}
- name: Setup Golang caches
uses: actions/cache@v3
with:
PATTERNS: |
**/**.go
go.mod
go.sum
- name: install
run: GOOS=linux GOARCH=${{ matrix.goarch }} go build
if: "env.GIT_DIFF != ''"
path: |
~/.cache/go-build
~/go/pkg/mod
key: ${{ runner.os }}-golang-${{ matrix.go }}-${{ hashFiles('**/go.sum') }}
restore-keys: |
${{ runner.os }}-golang-${{ matrix.go }}-
- name: Create coverage report and run tests
run: |
set -euo pipefail
GODEBUG=netdns=cgo go test -p 1 -json ./... -mod=readonly -timeout 8m -race -coverprofile=coverage.txt -covermode=atomic 2>&1 | tee test_results.json
- name: Sanitize test results
# We're utilizing `tee` above which can capture non-json stdout output so we need to remove non-json lines before additional parsing and submitting it to the external github action.
if: ${{ always() && env.TARGET_GOLANG_VERSION == matrix.go }}
run: cat test_results.json | jq -c -R 'fromjson? | select(type == "object")' > tmp.json && mv tmp.json test_results.json
- name: Output test failures
# Makes it easier to find failed tests so no need to scroll through the whole log.
if: ${{ failure() && env.TARGET_GOLANG_VERSION == matrix.go }}
run: |
jq --argjson fail_tests "$(jq -c -r 'select(.Action == "fail") | select(.Test) | .Test' test_results.json | jq -R -s -c -r 'split("\n") | map(select(length>0))')" 'select(.Test as $t | ($fail_tests | arrays)[] | select($t == .)) | select(.Output) | .Output' test_results.json | jq -r | sed ':a;N;$!ba;s/\n\n/\n/g' > test_failures.json
cat test_failures.json
exit 1
- name: Upload test results
if: ${{ always() && env.TARGET_GOLANG_VERSION == matrix.go }}
uses: actions/upload-artifact@v3
with:
name: test-results
path: |
test_*.json
- name: Annotate tests on GitHub
# Only annotate if the test failed on target version to avoid duplicated annotations on GitHub.
if: ${{ always() && env.TARGET_GOLANG_VERSION == matrix.go }}
uses: guyarb/[email protected]
with:
test-results: test_results.json
- name: Upload coverage to Codecov
if: ${{ always() && env.TARGET_GOLANG_VERSION == matrix.go }}
uses: codecov/codecov-action@v3
with:
files: ./coverage.txt
- name: golangci-lint
if: ${{ always() && env.TARGET_GOLANG_VERSION == matrix.go }}
uses: golangci/golangci-lint-action@v3
with:
version: latest
args: --timeout=10m
skip-cache: true
only-new-issues: true

tests:
build:
runs-on: ubuntu-latest
needs: build
needs: tests
strategy:
fail-fast: false
matrix:
goarch: ["amd64"]
goarch: ["arm64", "amd64"]
go: ["1.18"]
timeout-minutes: 5
name: Build for ${{ matrix.goarch }}
steps:
- uses: actions/setup-go@v2
- uses: actions/setup-go@v3
with:
go-version: "1.15"
- uses: actions/checkout@v2
go-version: ${{ matrix.go }}
- uses: actions/checkout@v3
- uses: technote-space/get-diff-action@v4
with:
PATTERNS: |
**/**.go
go.mod
go.sum
- name: Set up Go
uses: actions/setup-go@v2
with:
go-version: 1.15
- name: test & coverage report creation
run: |
GOARCH=${{ matrix.goarch }} go test -mod=readonly -timeout 8m -race -coverprofile=coverage.txt -covermode=atomic
if: env.GIT_DIFF
- uses: codecov/[email protected]
with:
file: ./coverage.txt
- name: Go build
run: GOOS=linux GOARCH=${{ matrix.goarch }} go build
if: env.GIT_DIFF
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
testdata/
Loading

0 comments on commit cc555d9

Please sign in to comment.