GSA / site-scanning

The central repository for the Site Scanning program
https://digital.gov/site-scanning
11 stars 2 forks source link

Add code linting to test workflow #125

Closed wesley-dean-gsa closed 2 years ago

wesley-dean-gsa commented 2 years ago

As a developer, so that I have clean code consistent in quality and formatting with established industry best practices, I would like a "linting" process added to our automated workflow.

Currently, neither the test nor deploy GitHub actions perform code linting -- they only perform unit tests (which is great!!). It would help the team deliver higher quality code that's less susceptible to bugs if we included additional code tests (e.g., megalinter).

wesley-dean-gsa commented 2 years ago

An additional job, lint, could be added to the workflow immediately before the build job so that subsequent jobs and steps use the clean, updated code.

https://github.com/GSA/site-scanning-engine/blob/d0cba746e2f996f48e7652d3642f3dd02221ce0c/.github/workflows/test.yml#L19

wesley-dean-gsa commented 2 years ago

:warning: adding a linting step may result in builds failing when they used to succeed. This is expected -- if there's a code issue, we want to be notified and have the process stop so that we can remedy it.

Therefore, it's recommended that we run whatever linting process we intend to use offline and cleanup and detected issues before incorporating this into the deployment process. Not doing so may make it difficult and/or inconvenient to affect a timely deployment, especially if we need to push a hot fix, patch a security concern, etc..

wesley-dean-gsa commented 2 years ago

This is the output from running MegaLinter locally:

+----SUMMARY-----+--------------------------+---------------+-------+-------+--------+--------------+
| Descriptor     | Linter                   | Mode          | Files | Fixed | Errors | Elapsed time |
+----------------+--------------------------+---------------+-------+-------+--------+--------------+
| ✅ ACTION      | actionlint               | list_of_files |     2 |       |      0 |        0.07s |
| ✅ BASH        | bash-exec                | file          |     2 |       |      0 |        0.02s |
| ✅ BASH        | shellcheck               | file          |     2 |       |      0 |        0.25s |
| ◬ BASH         | shfmt                    | file          |     2 |       |      2 |        0.06s |
| ❌ COPYPASTE   | jscpd                    | project       |   n/a |       |      2 |        3.13s |
| ✅ CREDENTIALS | secretlint               | project       |   n/a |       |      0 |        2.87s |
| ❌ DOCKERFILE  | dockerfilelint           | file          |     3 |       |      1 |         1.0s |
| ❌ DOCKERFILE  | hadolint                 | file          |     3 |       |      1 |        1.12s |
| ✅ GIT         | git_diff                 | project       |   n/a |       |      0 |        0.02s |
| ❌ JAVASCRIPT  | eslint                   | list_of_files |     1 |       |      1 |         2.7s |
| ❌ JAVASCRIPT  | standard                 | file          |     1 |       |      1 |        2.13s |
| ✅ JSON        | eslint-plugin-jsonc      | list_of_files |    25 |       |      0 |        5.86s |
| ✅ JSON        | jsonlint                 | file          |    25 |       |      0 |        9.51s |
| ◬ JSON         | prettier                 | list_of_files |    25 |       |      1 |        4.34s |
| ✅ JSON        | v8r                      | file          |    25 |       |      0 |       17.94s |
| ◬ MARKDOWN     | markdownlint             | list_of_files |     5 |       |     64 |        0.45s |
| ❌ MARKDOWN    | markdown-link-check      | file          |     5 |       |      4 |        3.61s |
| ✅ MARKDOWN    | markdown-table-formatter | list_of_files |     5 |       |      0 |        0.36s |
| ✅ PYTHON      | bandit                   | list_of_files |     1 |       |      0 |        1.05s |
| ◬ PYTHON       | black                    | list_of_files |     1 |       |      1 |        0.71s |
| ❌ PYTHON      | flake8                   | list_of_files |     1 |       |     10 |        0.57s |
| ◬ PYTHON       | isort                    | list_of_files |     1 |       |      1 |        0.35s |
| ✅ PYTHON      | mypy                     | list_of_files |     1 |       |      0 |        0.71s |
| ✅ PYTHON      | pylint                   | list_of_files |     1 |       |      0 |        3.67s |
| ❌ SPELL       | misspell                 | list_of_files |   141 |       |      1 |        0.23s |
| ❌ TYPESCRIPT  | eslint                   | list_of_files |    90 |       |      1 |        1.78s |
::set-output name=has_updated_sources::0
| ❌ TYPESCRIPT  | standard                 | file          |    90 |       |     87 |      199.84s |
| ◬ YAML         | prettier                 | list_of_files |     6 |       |      1 |         1.9s |
| ❌ YAML        | v8r                      | file          |     6 |       |      1 |        3.98s |
| ❌ YAML        | yamllint                 | list_of_files |     6 |       |      1 |         0.4s |
+----------------+--------------------------+---------------+-------+-------+--------+--------------+
wesley-dean-gsa commented 2 years ago

Add the following token: GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}

Add the following file to .github/workflows/mega-linter.yml: https://raw.githubusercontent.com/megalinter/megalinter/main/TEMPLATES/mega-linter.yml

Create a file in the project root: .mega-linter.yml

 don't use Mega-Linter to test the reports Mega-Linter created
ADDITIONAL_EXCLUDED_DIRECTORIES: report

# don't scan files listed in .gitignore (e.g., node_modules)
IGNORE_GITIGNORED_FILES: true

# don't attempt to apply fixes at this time
APPLY_FIXES: none

# please don't spell check everything...
DISABLE_LINTERS: SPELL_MISSPELL, COPYPASTE_JSCPD, SPELL_CSPELL

# only scan new / updated files, not everything
VALIDATE_ALL_CODEBASE: false

# don't print the alpaca -- it's cute, but we don't need it in the logs
PRINT_ALPACA: false

# don't fail on finding (yet)
DISABLE_ERRORS: true

Add to the .gitignore:

# don't include in commits files created by Mega-Linter
report/
mega-linter.log
wesley-dean-gsa commented 2 years ago

PR 142 (currently a draft PR) successfully lints commits as they're pushed.

Notes:

  1. regardless of whether the linter finds any issues or not, it will not block a PR from being merged. That is, it always returns success. This is a result of intentional configuration. A future issue will be used to enable success / fail.
  2. for instances where fixes could be automatically applied, that functionality has been intentionally disabled by configuration. A future issue will be used to enable automatic fixes
  3. one may see the results of the linting on a PR. Here, the results are published along with file counts, error counts, and links to scan results. A sample run may be seen here.
wesley-dean-gsa commented 2 years ago

This is resolved by GSA/site-scanning-engine#142