diff --git a/.trunk/trunk.yaml b/.trunk/trunk.yaml index 2560edf1d..ff8679539 100644 --- a/.trunk/trunk.yaml +++ b/.trunk/trunk.yaml @@ -20,7 +20,28 @@ plugins: ref: v1.0.4 lint: - # enabled linters inherited from github.com/trunk-io/configs plugin + files: + - name: plugin.yaml + filenames: [plugin.yaml] + definitions: + - name: definition-checker + description: Checks plugin.yaml files for repo best practices + files: [plugin.yaml] + runtime: node + extra_packages: + - ts-node + - yaml + commands: + - name: check + run: ts-node ${workspace}/repo-tools/definition-checker/check.ts ${target} + batch: true + sandbox_type: expanded + output: regex + parse_regex: "((?P.*) \\[(?P.*)\\]: (?P.*) \\((?P.*)\\))" + success_codes: [0] + enabled: + # enabled linters inherited from github.com/trunk-io/configs plugin + - definition-checker disabled: - pylint # pylint diagnostics are too strict - semgrep diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index c2c767cd2..2d8735670 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -1,18 +1,24 @@ # Contribution -Thanks for contributing to trunk's default plugins! Read on to learn more. +Thanks for contributing to Trunk's default plugins! Read on to learn more. +- [Prerequisites](#prerequisites) - [Overview](#overview) -- [Release process](#releases) - [Adding new linters](#linters) -- [Adding new actions](#actions) - [Adding new tools](#tools) +- [Adding new actions](#actions) +- [Release process](#releases) - [Guidelines](#guidelines) - [Docs](https://docs.trunk.io) +## Prerequisites + +1. Please [install the trunk CLI](https://docs.trunk.io/check/usage#install-the-cli) +2. Run `trunk check` in this repo to quickstart git-hooks and codegen + ## Overview -We use this repository to provide our users with default linters, actions, and tools. Trunk +We use this repository to provide our users with default linters, tools, and actions. Trunk automatically adds the following to users' trunk.yaml: ```yaml @@ -32,43 +38,24 @@ plugins: local: ``` -Adding a plugin source lets users run `trunk check enable` or `trunk actions enable` with linters -and actions defined in that plugin. For more information, see our +Adding a plugin source lets users run `trunk check enable`, `trunk tools enable`, or +`trunk actions enable` with definitions in that plugin. For more information, see our [docs](https://docs.trunk.io/docs/plugins). +**Please review our [Testing Guide](tests/README.md) for info on writing and running tests.** + If you have questions, please [stop by our community Slack](https://slack.trunk.io/), and if you have a feature request, you can [file it here](https://features.trunk.io/). -## Releases - -`trunk-io/plugins` is released on a fairly regular cadence that is independent of PRs. Users will -pick up these configuration changes by running `trunk upgrade` to automatically update their plugin -version to the latest release. - -```yaml -plugins: - sources: - - id: trunk - uri: https://github.com/trunk-io/plugins - ref: v1.2.5 # will change to the latest release on next `trunk upgrade` -``` - -We recommend only setting the above `ref` field to be our released tags, but if you need a linter or -action that hasn't been released yet, you can set `ref` to be a git SHA. Do **not** set `ref` to be -a branch name, or `HEAD`, as users will observe buggy behavior. - -Note that the ref and the cli version in the trunk.yaml must be compatible. This is managed by -`required_trunk_version`, as specified in [`plugin.yaml`](plugin.yaml). Users will not be able to -load a plugin source until they have upgraded to a compliant CLI version. - ## Linters To add a new linter: 1. Run `trunk check` to start up Trunk in the background. -2. Create a directory inside `linters/` with the name of your new linter. -3. Inside this new directory, create the following structure. Most of these files will be - automatically created for you: +2. Run `mkdir linters/` to start. This should autopopulate with a sample + [plugin.yaml](./repo-tools/linter-test-helper/linter_sample_plugin.yaml) and + [test file](./repo-tools/linter-test-helper/linter_sample.test.ts). If necessary, add them + yourself: ```text linters/ @@ -78,25 +65,57 @@ To add a new linter: │ README.md (optional) │ my-config.json (optional) └─test_data/ - └─basic.in.py (with appropriate extension) + └─basic.in.foo (with appropriate extension) ``` -4. Add your linter definition to `plugin.yaml` (consult the docs for [custom linters] and [custom +3. Add your linter definition to `plugin.yaml` (consult the docs for [custom linters] and [custom parsers] to understand how it should be defined). Most linters in this repository are defined as tools as well, so that they can be easily run manually from the command line. -5. Making sure the plugin in [`.trunk/trunk.yaml`](.trunk/trunk.yaml) is pointing to your local - repository, run `trunk check enable ` to enable your linter, and run `trunk check` to - verify that the configuration is valid and that you get desired diagnostics. Running - `trunk check --verbose` can help provide greater insights when debugging. -6. Add a few simple test cases to `my_linter.test.ts` to exercise your linter and generate - snapshots. Refer to [Testing Guidelines](tests/README.md) for more information on writing and - running tests. -7. Run `trunk check` to lint your changes. -8. Open a PR! +4. Run `trunk check enable ` to enable your linter, and run `trunk check` to verify that + the configuration is valid and that you get desired diagnostics. Running `trunk check --verbose` + can help provide greater insights when debugging. You may also wish to run on your test data, + i.e. `trunk check --verbose --force linters//test_data/basic.in.foo`. +5. Add a few simple test cases to `my_linter.test.ts` to exercise your linter and generate + snapshots. **Refer to the [Testing Guide](tests/README.md) for more information on writing, + running, and debugging tests.** +6. Revert any `.trunk/trunk.yaml` changes, and run `trunk check` to lint your changes. +7. Open a PR! [custom linters]: https://docs.trunk.io/check/custom-linters [custom parsers]: https://docs.trunk.io/check/custom-parsers +## Tools + +If the tool you intend to add functions primarily as a linter, please follow the instruction in +[linters](#linters). If it functions more as a standalone tool, please add it in the `tools/` +directory and follow the instructions below. + +To add a new tool: + +1. Run `trunk check` to start up Trunk in the background. +2. Run `mkdir tools/` to start. This should autopopulate with a sample + [plugin.yaml](./repo-tools/tool-test-helper/tool_sample_plugin.yaml) and + [test file](./repo-tools/tool-test-helper/tool_sample.test.ts). If necessary, add them yourself: + + ```text + tests/ + └─my-tool/ + │ plugin.yaml + │ my_tool.test.ts + └─README.md (optional) + ``` + +3. Add your tool definition to `plugin.yaml` (consult the docs for + [custom tools](https://docs.trunk.io/tools/configuration#tool-definitions) to understand how it + should be defined). +4. Run `trunk tools enable ` to enable your tool, and run its shim(s) from + `.trunk/tools/`. +5. Add a `toolInstallTest` to `my_tool.test.ts` to verify your tool's installation. If neccessary, + use `toolTest` instead. **Refer to the [Testing Guide](tests/README.md) for more information on + writing, running, and debugging tests.** +6. Revert any `.trunk/trunk.yaml` changes, and run `trunk check` to lint your changes. +7. Open a PR! + ## Actions To add a new action: @@ -118,54 +137,44 @@ To add a new action: `trunk actions enable ` to enable your linter, and run `trunk run ` to verify that the configuration is valid and that you get desired results. Running `trunk actions history ` can help provide greater insights when debugging. -6. We have not yet defined a testing framework for plugin actions, but we are working to add one - soon! Please briefly document your action in its README.md and in your PR. +6. Please briefly document your action in its README.md as appropriate. 7. Run `trunk check` to lint your changes. 8. Open a PR! -[actions]: https://docs.trunk.io/actions +Testing for Trunk Actions in this repo is in early development, so we do not require testing for new +action definitions. -## Tools +[actions]: https://docs.trunk.io/actions -If the tool you intend to add functions primarily as a linter, please follow the instruction in -[linters](#linters). If it functions more as a standalone tool, please add it in the `tools/` -directory and follow the instructions below. +## Releases -To add a new tool: +`trunk-io/plugins` is released on every few weeks. Users will pick up these configuration changes by +running `trunk upgrade` to automatically update their plugin version to the latest release. -1. Run `trunk check` to start up Trunk in the background. -2. Create a directory inside `tools/` with the name of your new linter. -3. Inside this new directory, create the following structure. Most of these files will be - automatically created for you: +```yaml +plugins: + sources: + - id: trunk + uri: https://github.com/trunk-io/plugins + ref: v1.2.5 # will change to the latest release on next `trunk upgrade` +``` - ```text - tests/ - └─my-tool/ - │ plugin.yaml - │ my_tool.test.ts - └─README.md (optional) - ``` +We recommend only setting the above `ref` field to be our released tags, but if you need a +linter/tool/action that hasn't been released yet, you can set `ref` to be a git SHA. Do **not** set +`ref` to be a branch name, as plugin branches are not refreshed. -4. Add your tool definition to `plugin.yaml` (consult the docs for - [custom tools](https://docs.trunk.io/tools/configuration#tool-definitions) to understand how it - should be defined). -5. Making sure the plugin in [`.trunk/trunk.yaml`](.trunk/trunk.yaml) is pointing to your local - repository, run `trunk tools enable ` to enable your tool, and access its shim(s) to run - it from `.trunk/tools/`. -6. Add a `toolInstallTest` to `my_tool.test.ts` to verify your tool's installation. If neccessary, - use `toolTest` instead. Refer to [Testing Guidelines](tests/README.md) for more information on - writing and running tests. -7. Run `trunk check` to lint your changes. -8. Open a PR! +Note that the ref and the cli version in the trunk.yaml must be compatible. This is managed by +`required_trunk_version`, as specified in [`plugin.yaml`](plugin.yaml). Users will not be able to +load a plugin source until they have upgraded to a compliant CLI version. ## Guidelines Please follow the guidelines below when contributing: - After defining a new linter or action, please add it to [`README.md`](README.md). -- If you run into any problems while defining new linters or actions, feel free to reach out on our - [Slack](https://slack.trunk.io/). We are continuously working to improve the process of - integrating with trunk, and all feedback is appreciated! +- If you run into any problems while defining new linters/tools/actions, feel free to reach out on + our [Slack](https://slack.trunk.io/). We are continuously working to improve the process of + integrating with Trunk, and all feedback is appreciated! ## Development diff --git a/README.md b/README.md index 641abe235..671649c0d 100644 --- a/README.md +++ b/README.md @@ -3,14 +3,19 @@ [![Trunk.io](https://static.trunk.io/assets/trunk_plugins_logo.png)](https://trunk.io) [![docs](https://img.shields.io/badge/-docs-darkgreen?logo=readthedocs&logoColor=ffffff)][docs] +[![contributing](https://img.shields.io/badge/contributing-darkgreen?logo=readthedocs&logoColor=ffffff)][contributing] +[![testing guide](https://img.shields.io/badge/testing_guide-darkgreen?logo=readthedocs&logoColor=ffffff)][testing guide] [![slack](https://img.shields.io/badge/-slack-611f69?logo=slack)][slack] [![vscode](https://img.shields.io/visual-studio-marketplace/i/trunk.io?color=0078d7&label=vscode&logo=visualstudiocode)][vscode] [![openssf](https://api.securityscorecards.dev/projects/github.com/trunk-io/plugins/badge)](https://api.securityscorecards.dev/projects/github.com/trunk-io/plugins) +[testing guide]: ./tests/README.md +[contributing]: ./CONTRIBUTING.md + ### Welcome This repository is the official [Trunk.io](https://trunk.io/) repo containing Trunk's integrations -for linters, formatters, security tools, githooks, and default configs. By default, all trunk users +for linters, formatters, security tools, githooks, and default configs. By default, all Trunk users import this repo as a plugin, via this snippet in `.trunk/trunk.yaml`: ```yaml @@ -18,12 +23,11 @@ plugins: sources: - id: trunk uri: https://github.com/trunk-io/plugins - ref: v1.4.4 + ref: v1.5.0 ``` -This repo is open to contributions! See our -[contribution guidelines](https://github.com/trunk-io/plugins/blob/main/CONTRIBUTING.md) and join -our [slack community][slack] for help. **If you're adding new tools, please see our +This repo is open to contributions! See our [contribution guidelines](CONTRIBUTING.md) and join our +[slack community][slack] for help. **If you're adding new tools, please see our [testing guide](tests/README.md) as well!** ### Supported Linters, Formatters, and Security Tools diff --git a/jest.config.json b/jest.config.json index 138e05427..6d5d41dac 100644 --- a/jest.config.json +++ b/jest.config.json @@ -2,5 +2,6 @@ "preset": "ts-jest", "modulePaths": [""], "setupFilesAfterEnv": ["/tests/jest_setup.ts"], - "reporters": ["/tests/reporter/index.js", "default"] + "reporters": ["/tests/reporter/index.js", "default"], + "testPathIgnorePatterns": ["/node_modules/", "/repo-tools/"] } diff --git a/linters/pragma-once/pragma_once.test.ts b/linters/pragma-once/pragma_once.test.ts index 2d5dfd16e..c339a2131 100644 --- a/linters/pragma-once/pragma_once.test.ts +++ b/linters/pragma-once/pragma_once.test.ts @@ -1,4 +1,4 @@ import { linterFmtTest } from "tests"; -// TODO(Tyler): We will eventually need to add a couple more test cases involving failure modes and other autofixes. +// A simple formatter test to run 'pragma-once' on 'test_data/basic.in.hh' linterFmtTest({ linterName: "pragma-once" }); diff --git a/linters/ruff/plugin.yaml b/linters/ruff/plugin.yaml index ad2d5b9ad..d2cb7a7e3 100644 --- a/linters/ruff/plugin.yaml +++ b/linters/ruff/plugin.yaml @@ -64,6 +64,7 @@ lint: run: ruff --version - name: ruff-nbqa + description: A Python linter for Jupyter notebooks files: [jupyter] commands: - name: lint diff --git a/linters/sort-package-json/plugin.yaml b/linters/sort-package-json/plugin.yaml index e660659fd..dbfbda8d1 100644 --- a/linters/sort-package-json/plugin.yaml +++ b/linters/sort-package-json/plugin.yaml @@ -13,6 +13,7 @@ lint: definitions: - name: sort-package-json + description: Sorts package.json keys files: [package-json] commands: - name: format diff --git a/linters/sqlfluff/sqlfluff.test.ts b/linters/sqlfluff/sqlfluff.test.ts index 1019e9593..6a8c24177 100644 --- a/linters/sqlfluff/sqlfluff.test.ts +++ b/linters/sqlfluff/sqlfluff.test.ts @@ -1,5 +1,6 @@ import { linterCheckTest, linterFmtTest, TestCallback } from "tests"; +// A simple test to run 'sqlfluff lint' // basic_check.out.json is the result of running: // trunk check linters/sqlfluff/test/basic_check.in.sql --force --filter=sqlfluff --output=json linterCheckTest({ linterName: "sqlfluff", namedTestPrefixes: ["basic_check"] }); @@ -11,10 +12,10 @@ const fmtCallbacks: TestCallback = (driver) => { const sqlfluffRegex = /- sqlfluff@(.+)\n/; const newContents = currentContents.replace( sqlfluffRegex, - "- sqlfluff@$1:\n commands: [lint, fix]\n" + "- sqlfluff@$1:\n commands: [lint, fix]\n", ); driver.writeFile(trunkYamlPath, newContents); }; -// TODO(Tyler): We will eventually need to add a couple more test cases involving failure modes. +// An additional test to run 'sqlfluff fmt' with some additional test setup. linterFmtTest({ linterName: "sqlfluff", namedTestPrefixes: ["basic_fmt"], preCheck: fmtCallbacks }); diff --git a/linters/trufflehog/plugin.yaml b/linters/trufflehog/plugin.yaml index b3952118e..1a293e212 100644 --- a/linters/trufflehog/plugin.yaml +++ b/linters/trufflehog/plugin.yaml @@ -49,6 +49,7 @@ lint: # Variant of trufflehog that scans git history. - name: trufflehog-git + description: Scan git history for secrets files: [ALL] download: trufflehog known_good_version: 3.59.0 diff --git a/repo-tools/definition-checker/check.ts b/repo-tools/definition-checker/check.ts new file mode 100644 index 000000000..64ffaf2f8 --- /dev/null +++ b/repo-tools/definition-checker/check.ts @@ -0,0 +1,149 @@ +import fs from "fs"; +import path from "path"; +// trunk-ignore(eslint/import/no-extraneous-dependencies) +import YAML from "yaml"; + +// Avoid strictly typing composite config +// trunk-ignore-all(eslint/@typescript-eslint/no-unsafe-assignment) +// trunk-ignore-all(eslint/@typescript-eslint/no-unsafe-member-access) +// trunk-ignore-all(eslint/@typescript-eslint/no-unsafe-call) +// trunk-ignore-all(eslint/@typescript-eslint/no-unsafe-return) + +/**** Helpers ****/ + +/** + * Generate a diagnostic message that matches the parse_regex spec. + */ +const generateMessage = (file: string, message: string, code: string): string => + `${file} [error]: ${message} (${code})`; + +/** + * Validate that a plugin.yaml doesn't explicitly enable any linters, tools, or actions. + */ +const validateEnables = (file: string, config: any): string[] => { + const enableErrors: string[] = []; + const lintEnabled = config.lint?.enabled; + const toolsEnabled = config.tools?.enabled; + const actionsEnabled = config.actions?.enabled; + + [ + [lintEnabled, "Linter"], + [toolsEnabled, "Tool"], + [actionsEnabled, "Action"], + ].forEach((value: any[][]) => { + const [enableds, enabledType] = value as [any[], string]; + if (enableds?.length) { + enableErrors.push( + ...enableds.map((enabled: string) => + generateMessage( + file, + `${enabledType} ${enabled} is explicitly enabled`, + `no-enable-${enabledType.toLowerCase()}`, + ), + ), + ); + } + }); + + const lintDefinitions = config.lint?.definitions; + const toolsDefinitions = config.tools?.definitions; + const actionsDefinitions = config.actions?.definitions; + + [ + [lintDefinitions, "Linter"], + [toolsDefinitions, "Tool"], + [actionsDefinitions, "Action"], + ].forEach((value: any[][]) => { + const [definitions, definitionType] = value as [any[], string]; + if (!definitions) { + return; + } + enableErrors.push( + ...definitions.reduce((acc: string[], definition: any) => { + if (definition?.enabled) { + acc.push( + generateMessage( + file, + `${definitionType} ${definition.name ?? definition.id} is explicitly enabled`, + `no-enable-${definitionType.toLowerCase()}`, + ), + ); + } + return acc; + }, []), + ); + }); + + return enableErrors; +}; + +/** + * Ensure that a linter definition has a 'suggest_if' and 'description' field. + */ +const validateLinters = (file: string, config: any): string[] => { + if (!config.lint?.definitions) { + return []; + } + + return config.lint.definitions.reduce((acc: string[], definition: any) => { + if (definition.deprecated) { + return acc; + } + if (!definition.suggest_if) { + acc.push( + generateMessage( + file, + `Linter ${definition.name} should specify 'suggest_if'`, + "suggest-if-linter", + ), + ); + } + if (!definition.description) { + acc.push( + generateMessage( + file, + `Linter ${definition.name} should specify 'description'`, + "description-linter", + ), + ); + } + return acc; + }, []); +}; + +/** + * Ensure that a plugin.yaml in the linters or tools subfolders has a matching test file. + */ +const validateTests = async (file: string): Promise => { + if (!file.includes("linters") || !file.includes("tools")) { + return []; + } + + const directoryContents = await fs.promises.readdir(path.dirname(file)); + const hasTest = directoryContents.some((dirFile: string) => dirFile.endsWith(".test.ts")); + if (hasTest) { + return []; + } + return [generateMessage(file, "No test file found", "no-test-file")]; +}; + +/**** Lint Plugin Files ****/ + +const fileArgs = process.argv.slice(2); + +const processFile = async (filePath: string) => { + const fileContent = await fs.promises.readFile(filePath, "utf8"); + const yamlContents = YAML.parse(fileContent); + const errors = validateEnables(filePath, yamlContents); + errors.push(...validateLinters(filePath, yamlContents)); + errors.push(...(await validateTests(filePath))); + console.log(errors.join("\n")); +}; + +const processFiles = async (filePaths: string[]) => { + for (const filePath of filePaths) { + await processFile(filePath); + } +}; + +void processFiles(fileArgs); diff --git a/repo-tools/linter-test-helper/generate b/repo-tools/linter-test-helper/generate index 9d4038f1c..0992fb654 100755 --- a/repo-tools/linter-test-helper/generate +++ b/repo-tools/linter-test-helper/generate @@ -5,60 +5,12 @@ from datetime import datetime, timedelta import click -INITIAL_PLUGIN_CONTENTS = """version: 0.1 -# Tools can be either runtime package-based or -# download-based. This boilerplate assumes the (far more verbose) latter case. -downloads: - - name: NAME_HERE - # executable: true - # NOTE: These are (common) sample values. Please replace with real values. - downloads: - - os: - linux: linux - macos: darwin - windows: windows - cpu: - x86_64: amd64 - arm_64: arm64 - url: https://URL_HERE/${version}/${os}-${cpu}-NAME_HERE -# Fill out this part if the linter is tool-based (most common case) - otherwise delete -tools: - definitions: - - name: NAME_HERE - # RUNTIME_OR_DOWNLOAD_INFO_HERE - # download: NAME_HERE - known_good_version: VERSION_HERE - shims: [NAME_HERE] -lint: - definitions: - - name: NAME_HERE - files: [FILE_TYPES_HERE] - # TOOL_OR_RUNTIME_OR_DOWNLOAD_INFO_HERE - known_good_version: # VERSION_HERE - suggest_if: never - commands: - - name: LINT_OR_FORMAT_HERE - output: OUTPUT_TYPE_HERE - run: COMMAND_HERE - success_codes: [0] -""" +directory = os.path.dirname(__file__) +with open(os.path.join(directory, "linter_sample_plugin.yaml"), "r") as file: + initial_plugin_contents = file.read() -INITIAL_TEST_CONTENTS = """import { linterCheckTest, linterFmtTest } from "tests"; -// Uncomment and use if your tool is a linter -// linterCheckTest({ linterName: "*{}*" }); - -// Uncomment and use if your tool is a formatter -// linterFmtTest({ linterName: "*{}*" }); - -// Guidelines for configuring tests: -// - By default, linters and formatters will only run on files with syntax `.in.` -// - You can customize test setup using the `preCheck` callback (see git_diff_check.test.ts and golangci_lint.test.ts) -// - You can specify additional customization using the `customLinterCheckTest and customLinterFmtTest` helpers -// - Additional information on test setup can be found in tests/README.md -// -// If you are unable to write a test for this linter, please document why in your PR, and add -// it to the list in tests/repo_tests/test_coverage_test.ts -""" +with open(os.path.join(directory, "linter_sample.test.ts"), "r") as file: + initial_test_contents = file.read() @click.group() @@ -86,7 +38,9 @@ def scan(workspace): generated_files = True # Write plugin.yaml with open(os.path.join(subdir_path, "plugin.yaml"), "w") as plugin_file: - plugin_file.write(INITIAL_PLUGIN_CONTENTS) + plugin_file.write( + initial_plugin_contents.replace("NAME_HERE", linter_name) + ) # Write test file with open( @@ -95,7 +49,7 @@ def scan(workspace): ), "w", ) as test_file: - test_file.write(INITIAL_TEST_CONTENTS.replace("*{}*", linter_name)) + test_file.write(initial_test_contents.replace("NAME_HERE", linter_name)) # Create empty test_data dir test_dir = os.path.join(subdir_path, "test_data") diff --git a/repo-tools/linter-test-helper/linter_sample.test.ts b/repo-tools/linter-test-helper/linter_sample.test.ts new file mode 100644 index 000000000..775f434e1 --- /dev/null +++ b/repo-tools/linter-test-helper/linter_sample.test.ts @@ -0,0 +1,15 @@ +import { linterCheckTest, linterFmtTest } from "tests"; + +// Guidelines for configuring tests: +// - By default, linters and formatters will only run on files with syntax `.in.` +// - You can customize test setup using the `preCheck` callback (see git_diff_check.test.ts and golangci_lint.test.ts) +// - You can specify additional customization using the `customLinterCheckTest` and `customLinterFmtTest` helpers +// - Additional information on test setup can be found in tests/README.md +// +// If you are unable to write a test for this linter, please document why in your PR. Feel free to ask for help! + +// Use if your tool is a linter +linterCheckTest({ linterName: "NAME_HERE" }); + +// Use if your tool is a formatter +linterFmtTest({ linterName: "NAME_HERE" }); diff --git a/repo-tools/linter-test-helper/linter_sample_plugin.yaml b/repo-tools/linter-test-helper/linter_sample_plugin.yaml new file mode 100644 index 000000000..620ad80a7 --- /dev/null +++ b/repo-tools/linter-test-helper/linter_sample_plugin.yaml @@ -0,0 +1,54 @@ +version: 0.1 +# Tools can be either runtime package-based or download-based. Modify this boilerplate +# to fit your use case. + +# Fill out this part if the linter/tool is download-based - otherwise delete +downloads: + - name: NAME_HERE + # executable: true + # DOWNLOAD_URL_INFO_HERE + downloads: + - os: + linux: linux + macos: darwin + windows: windows + cpu: + x86_64: amd64 + arm_64: arm64 + url: https://URL_HERE/${version}/${os}-${cpu}-NAME_HERE +# Fill out this part if the linter is tool-based (most common case) - otherwise delete +tools: + definitions: + - name: NAME_HERE + # RUNTIME_OR_DOWNLOAD_INFO_HERE + # runtime: RUNTIME_HERE + # package: NAME_HERE + # -- OR -- + # download: NAME_HERE + known_good_version: VERSION_HERE + shims: [NAME_HERE] + health_checks: + - command: NAME_HERE --version + parse_regex: ${semver} +lint: + definitions: + - name: NAME_HERE + files: [FILE_TYPES_HERE] + # TOOL_OR_RUNTIME_OR_DOWNLOAD_INFO_HERE + # tools: [NAME_HERE] + # -- OR -- + # runtime: RUNTIME_HERE + # package: NAME_HERE + # -- OR -- + # download: NAME_HERE + known_good_version: VERSION_HERE + suggest_if: never + commands: + # https://docs.trunk.io/check/configuration/custom-linters/commands/definition + - name: LINT_OR_FORMAT_HERE + # https://docs.trunk.io/check/configuration/custom-linters/commands/output-types + output: OUTPUT_TYPE_HERE + run: COMMAND_HERE + success_codes: [0] + # formatter: true + # in_place: true diff --git a/repo-tools/tool-test-helper/generate b/repo-tools/tool-test-helper/generate index 8c9f26f69..506726d4e 100644 --- a/repo-tools/tool-test-helper/generate +++ b/repo-tools/tool-test-helper/generate @@ -5,60 +5,12 @@ from datetime import datetime, timedelta import click -INITIAL_PLUGIN_CONTENTS = """version: 0.1 -# Tools can be either runtime package-based or -# download-based. This boilerplate assumes the (far more verbose) latter case. -downloads: - - name: *{}* - # executable: true - # NOTE: These are (common) sample values. Please replace with real values. - downloads: - - os: - linux: linux - macos: darwin - windows: windows - cpu: - x86_64: amd64 - arm_64: arm64 - url: https://URL_HERE/${version}/${os}-${cpu}-NAME_HERE -tools: - definitions: - - name: *{}* - # RUNTIME_OR_DOWNLOAD_INFO_HERE - # download: *{}* - known_good_version: VERSION_HERE - # NOTE: shim may differ from tool name - shims: [*{}*] -""" +directory = os.path.dirname(__file__) +with open(os.path.join(directory, "tool_sample_plugin.yaml"), "r") as file: + initial_plugin_contents = file.read() -INITIAL_TEST_CONTENTS = """import { makeToolTestConfig, toolTest, toolInstallTest } from "tests"; -toolTest({ - toolName: "*{}*", - toolVersion: "VERSION_HERE", - testConfigs: [ - makeToolTestConfig({ - command: ["SHIM_NAME", "COMMAND_HERE"], - expectedOut: "OUTPUT_HERE", - }), - ], -}); - -toolInstallTest({ - toolName: "*{}*", - toolVersion: "VERSION_HERE", -}) - -// Guidelines for configuring tests: -// - Prefer using health check in config + toolInstallTest, if you must use toolTest leave a -// comment explaining why. Only one of the two options is sufficient. -// - Usually, just a version or help text command is sufficient -// - add a test for each command that is used in the plugin.yaml -// - exit code 0 is assumed, so set expectedExitCode if it is different -// - expectedOut/expectedErr do a substring match, so you can just put a portion of the output -// -// If you are unable to write a test for this tool, please document why in your PR, and add -// it to the list in tests/repo_tests/test_coverage_test.ts -""" +with open(os.path.join(directory, "tool_sample.test.ts"), "r") as file: + initial_test_contents = file.read() @click.group() @@ -86,7 +38,9 @@ def scan(workspace): generated_files = True # Write plugin.yaml with open(os.path.join(subdir_path, "plugin.yaml"), "w") as plugin_file: - plugin_file.write(INITIAL_PLUGIN_CONTENTS.replace("*{}*", tool_name)) + plugin_file.write( + initial_plugin_contents.replace("NAME_HERE", tool_name) + ) # Write test file with open( @@ -95,7 +49,7 @@ def scan(workspace): ), "w", ) as test_file: - test_file.write(INITIAL_TEST_CONTENTS.replace("*{}*", tool_name)) + test_file.write(initial_test_contents.replace("NAME_HERE", tool_name)) print("Created starter files in {}", subdir_path) diff --git a/repo-tools/tool-test-helper/tool_sample.test.ts b/repo-tools/tool-test-helper/tool_sample.test.ts new file mode 100644 index 000000000..22c444378 --- /dev/null +++ b/repo-tools/tool-test-helper/tool_sample.test.ts @@ -0,0 +1,28 @@ +import { makeToolTestConfig, toolInstallTest, toolTest } from "tests"; + +// Guidelines for configuring tests: +// - Prefer using health check in config + toolInstallTest, if you must use toolTest leave a +// comment explaining why. Only one of the two options is sufficient. +// - Usually, just a version or help text command is sufficient +// - add a test for each command that is used in the plugin.yaml +// - exit code 0 is assumed, so set expectedExitCode if it is different +// - expectedOut/expectedErr do a substring match, so you can just put a portion of the output +// +// If you are unable to write a test for this tool, please document why in your PR. Feel free to ask for help! + +toolInstallTest({ + toolName: "NAME_HERE", + toolVersion: "VERSION_HERE", +}); + +// No need to include if a toolInstallTest is included. +toolTest({ + toolName: "NAME_HERE", + toolVersion: "VERSION_HERE", + testConfigs: [ + makeToolTestConfig({ + command: ["SHIM_NAME", "COMMAND_HERE"], + expectedOut: "OUTPUT_HERE", + }), + ], +}); diff --git a/repo-tools/tool-test-helper/tool_sample_plugin.yaml b/repo-tools/tool-test-helper/tool_sample_plugin.yaml new file mode 100644 index 000000000..185bea871 --- /dev/null +++ b/repo-tools/tool-test-helper/tool_sample_plugin.yaml @@ -0,0 +1,31 @@ +version: 0.1 +# Tools can be either runtime package-based or download-based. Modify this boilerplate +# to fit your use case. + +# Fill out this part if the linter/tool is download-based - otherwise delete +downloads: + - name: NAME_HERE + # executable: true + # DOWNLOAD_URL_INFO_HERE + downloads: + - os: + linux: linux + macos: darwin + windows: windows + cpu: + x86_64: amd64 + arm_64: arm64 + url: https://URL_HERE/${version}/${os}-${cpu}-NAME_HERE +tools: + definitions: + - name: NAME_HERE + # RUNTIME_OR_DOWNLOAD_INFO_HERE + # runtime: RUNTIME_HERE + # package: NAME_HERE + # -- OR -- + # download: NAME_HERE + known_good_version: VERSION_HERE + shims: [NAME_HERE] + health_checks: + - command: NAME_HERE --version + parse_regex: ${semver} diff --git a/tests/README.md b/tests/README.md index 4db7c70ee..351013159 100644 --- a/tests/README.md +++ b/tests/README.md @@ -1,16 +1,25 @@ # Testing -To run tests and generate snapshots, run `npm install` and `npm test `. +```bash +npm ci +npm test +``` + +- [Overview](#overview) +- [Configuring Tests](#configuring-tests) +- [Running and Debugging Tests](#running-tests) +- [Additional Options](#additional-options) ## Overview -We ask that all new linter definitions in this repository add some basic testing. This should be a -straightforward and simple process, with minimal overhead, but let us know if you need help! Please -start by following the instructions below: +We ask that all new linter and tool definitions in this repository add some basic testing. This +should be a straightforward and simple process, with minimal overhead, but let us know if you need +help! Please start by following the instructions below: ## Configuring Tests -Please create a directory structure in your linter/formatter definition analogous to the following: +Follow the setup quickstart in the [contributing guidelines](../CONTRIBUTING.md). As a result, you +should have a directory structure like the following: ```text linters/ @@ -20,16 +29,17 @@ linters/ │ README.md (optional) │ my-config.json (optional) └─test_data/ - └─basic.in.py (with appropriate extension) + └─basic.in.foo (with appropriate extension) ``` - Specify a `README.md` if your linter integration requires additional explanation or configuration. -- Specify a `my-config.json` (or whatever `direct_configs` item applies) ONLY if providing this - config file is sufficient to enable your linter in ALL cases. This will be created whenever - someone enables your linter. -- Specify a typescript test file that calls `linterCheckTest` or `linterFmtTest` with the name of - your linter and (optionally) the prefixes of your input files and any special callbacks. -- Inside of `test_data/`, provide at least one input file. +- Specify a `my-config.json` (or whatever `direct_configs` item applies) ONLY if it is universally + applicable. This will be created whenever someone enables your linter. +- Specify a [Jest test file](../repo-tools/linter-test-helper/linter_sample.test.ts) that calls + `linterCheckTest` or `linterFmtTest` with the name of your linter and (optionally) the prefixes of + your input files and any special callbacks. +- Inside of `test_data/`, provide at least one input file. **This file should be named + `.in.` to be automatically picked up by the testing framework.** - For linters, specify a sample input file (with an appropriate file extension). For reference, the tests will run the following command against your input file: @@ -45,26 +55,21 @@ linters/ trunk fmt --force --filter= ``` -Refer to [sqlfluff](../linters/sqlfluff) or [pragma-once](../linters/pragma-once) as testing -examples. +Refer to [sqlfluff](../linters/sqlfluff/sqlfluff.test.ts) or +[pragma-once](../linters/pragma-once/pragma_once.test.ts) as testing examples. ## Running Tests -To run all tests, run `npm install` and then run: +The first time you run a test, it will automatically generate the necessary snapshot for the +`known_good_version`. -```bash -npm test -``` - -To run an individual test, run: +To run a test with debug information and preserve a sandbox for debugging, run: ```bash -npm test +npm ci +SANDBOX_DEBUG="true" DEBUG="Driver:*" npm test ``` -Then, verify that the generated snapshot file includes the results you would expect (e.g. an Object -with several fileIssues, no taskFailures). - For context, the general test execution is as follows: 1. Create a sandbox testing directory by copying a linter's subdirectory and its `test_data`. @@ -72,6 +77,7 @@ For context, the general test execution is as follows: [.trunk/trunk.yaml](../.trunk/trunk.yaml). 3. Run `trunk check enable `. 4. Run `trunk check` or `trunk fmt` on files with the `.in.` syntax. +5. Cache any linter/tool downloads in `${TMPDIR:-/tmp}/plugins_testing_download_cache` ### Linter Versioning @@ -99,7 +105,7 @@ The process of resolving snapshots for asserting output correctness is as follow If no such snapshot exists, a new snapshot is created with the enabled version of the linter (use [debug logging](#debugging) to see what version was enabled). -The reasoning for this setup is threefold: +[The reasoning for this setup is threefold](https://trunk.io/blog/how-we-eliminate-tool-rot-and-confidently-upgrade-our-open-source-dependencies): 1. Linters can update their arguments or outputs on occasion, which can lead to a different trunk output. We would like to be aware of these changes, particularly if they require trunk to accept @@ -142,42 +148,25 @@ include: version change. - `SANDBOX_DEBUG` if `true`, prevents sandbox test directories from being deleted, and logs their path for additional debugging. +- [`DEBUG`](https://www.npmjs.com/package/debug) is used to configure log information. Use + `DEBUG=Driver:*` to view useful test driver logs, or use `DEBUG=*:sqlfluff*` to view all logs + related to a particular linter (`::<#>`). ### CI PRs will run 5 types of tests across all platforms as applicable: -1. Enable and test all linters with their `known_good_version`, if applicable. To replicate this - behavior, run: `PLUGINS_TEST_LINTER_VERSION=KnownGoodVersion npm test`. If the +1. Enable and test all changed linters with their `known_good_version`, if applicable. To replicate + this behavior, run: `PLUGINS_TEST_LINTER_VERSION=KnownGoodVersion npm test`. If the `known_good_version` is different from the version enabled when you defined the linter, you will need to first run this locally to generate a snapshot file. -2. Enable and test all linters with their latest version, if applicable. To replicate this behavior, - run: `npm test`. -3. Assert that all linters pass config validation. This is also validated while running: `npm test`. -4. Assert that all linters have test coverage. -5. Assert that all linters are included in the [`README.md`](../README.md). +2. Enable and test all changed linters with their latest version, if applicable. To replicate this + behavior, run: `npm test`. +3. Assert that all linters pass config validation and best practices. +4. Assert that all linters are included in the [`README.md`](../README.md). ### Debugging Individual tests normally complete in less than 1 minute. They may take up to 5 minutes or so if the -dependency cache is empty (linters need to be downloaded and installed to run the linter tests). -Subsequent runs will not experience this delay. - -Errors encountered during test runs are reported through the standard `console`, but additional -debugging is provided using [debug](https://www.npmjs.com/package/debug). The namespace convention -used is `::<#>`, where Location is `Driver | Tests`, linter is the name of the -linter being tested (alternatively `test<#>` if no linter is specified), and # is a counter used to -distinguish between multiple tests with the same linter. - -Accordingly, in order to view debug logs for all sqlfluff tests, you can set the environment -variable: - -```bash -DEBUG=*:sqlfluff* -``` - -To just see debug logs from the test driver, use: - -```bash -DEBUG=Driver:* -``` +`/tmp/plugins_testing_download_cache` dependency cache is empty (linters need to be downloaded and +installed to run the linter tests). Subsequent runs will not experience this delay. diff --git a/tests/repo_tests/config_check.test.ts b/tests/repo_tests/config_check.test.ts index bd364d848..028f1a063 100644 --- a/tests/repo_tests/config_check.test.ts +++ b/tests/repo_tests/config_check.test.ts @@ -7,7 +7,6 @@ import { osTimeoutMultiplier, REPO_ROOT } from "tests/utils"; // trunk-ignore-all(eslint/@typescript-eslint/no-unsafe-assignment) // trunk-ignore-all(eslint/@typescript-eslint/no-unsafe-member-access) // trunk-ignore-all(eslint/@typescript-eslint/no-unsafe-call) -// trunk-ignore-all(eslint/@typescript-eslint/no-unsafe-argument) // trunk-ignore-all(eslint/@typescript-eslint/no-unsafe-return) jest.setTimeout(300000 * osTimeoutMultiplier); @@ -173,75 +172,3 @@ describe("Global config health check", () => { `); }); }); - -/** - * This test validates that only explicitly enumerated linters are recommended by default. - */ -describe("Explicitly enabled healthcheck", () => { - // Step 1: Define test setup and teardown - const driver = setupLintDriver(REPO_ROOT, { - setupGit: false, - setupTrunk: true, - }); - - // Step 2: Validate that no plugin linters or actions are explicitly enabled - it("validate explicitly enabled actions and linters", async () => { - // Remove user.yaml if it exists, since it could affect the enabled set. - // Specifying force avoid errors being thrown if it doesn't exist. - fs.rmSync(path.resolve(driver.getSandbox(), ".trunk/user.yaml"), { - force: true, - }); - - const compositeConfig = await driver.getFullTrunkConfig(); - const lintDefinitions = compositeConfig.lint.definitions; - const actionDefinitions = compositeConfig.actions.definitions; - - const explicitlyEnabledLinters = lintDefinitions.reduce( - (enabledLinters: string[], definition: any) => { - if (definition.enabled) { - return enabledLinters.concat(definition.name); - } - - if (definition.commands) { - const commandEnabled = definition.commands.reduce((enabled: boolean, command: any) => { - if (command.enabled) { - return true; - } - return enabled; - }, false); - if (commandEnabled) { - return enabledLinters.concat(definition.name); - } - } - - return enabledLinters; - }, - [], - ); - - // No linters should be enabled by default - expect(explicitlyEnabledLinters).toMatchInlineSnapshot(`[]`); - - const explicitlyEnabledActions = actionDefinitions.reduce( - (enabledActions: string[], definition: any) => { - if (definition.enabled) { - return enabledActions.concat(definition.id); - } - - return enabledActions; - }, - [], - ); - - // Built-in actions only - expect(explicitlyEnabledActions).toMatchInlineSnapshot(` - [ - "trunk-cache-prune", - "trunk-share-with-everyone", - "trunk-single-player-auto-upgrade", - "trunk-single-player-auto-on-upgrade", - "trunk-whoami", - ] - `); - }); -}); diff --git a/tests/repo_tests/readme_inclusion.test.ts b/tests/repo_tests/readme_inclusion.test.ts index c6a6dd452..60462d941 100644 --- a/tests/repo_tests/readme_inclusion.test.ts +++ b/tests/repo_tests/readme_inclusion.test.ts @@ -12,6 +12,7 @@ const readmeTableContents = readmeContents.substring( ); const reducedReadmeContents = readmeTableContents ? readmeTableContents : readmeContents; +// TODO(Tyler): Move this to 'definition-checker' linter. // This test asserts that all linters are included in the root README.md. This does not cover subcommands, and it assumes one // directory per linter. Name mapping can be achieved through `abbreviationMapping`. describe("All linters must be included in README.md", () => { diff --git a/tests/repo_tests/test_coverage.test.ts b/tests/repo_tests/test_coverage.test.ts deleted file mode 100644 index a74e1d051..000000000 --- a/tests/repo_tests/test_coverage.test.ts +++ /dev/null @@ -1,50 +0,0 @@ -import { execSync } from "child_process"; -import fs from "fs"; -import path from "path"; -import { REPO_ROOT } from "tests/utils"; - -// This should be empty unless we've agreed that certain linters can omit coverage. e.g. ["oxipng"] -// Note that any linters without tests will not upload validated versions for `trunk upgrade`. -const excludedLinters: string[] = []; - -// This test asserts that all linters have at least one test. All new linters are expected to have -// test coverage. Review tests/README.md for testing guidelines. Prefer using npm test for indirection -// in this test so that we get an accurate list of all tests, regardless of any changes to the test spec -// in jest.config.json. -describe("All linters must have tests", () => { - // Find all tests detected by jest - const stdout = execSync("npm test -- --listTests", { cwd: REPO_ROOT }).toString(); - const testFiles = stdout - .split("\n") - .filter((file) => file.startsWith("/") || file.match(/[A-Z]:\\/)) - .map((file) => path.relative(REPO_ROOT, file)); - - // Key the tests by their linter subdirectory - const testDirMap = testFiles.reduce((accumulator: Map, file: string) => { - const linterSubdir = - process.platform === "win32" ? file.match(/linters\\[^\\]+/) : file.match(/linters\/[^/]+/); - if (linterSubdir) { - const matches = accumulator.get(linterSubdir[0]) ?? []; - accumulator.set(linterSubdir[0], [...matches, file]); - } - return accumulator; - }, new Map()); - const testDirObject = Object.fromEntries(testDirMap); - - // Find all linter subdirectories - const linterDir = path.resolve(REPO_ROOT, "linters"); - const linters = fs - .readdirSync(linterDir) - .filter((file) => fs.lstatSync(path.resolve(linterDir, file)).isDirectory()); - - // Assert that each linter subdirectory has a test (excluding an explicit subset) - linters - .filter((linter) => !excludedLinters.includes(linter)) - .forEach((linter) => { - // trunk-ignore(eslint/jest/valid-title) - it(linter, () => { - const linterSubdir = path.join("linters", linter); - expect(testDirObject).toHaveProperty(linterSubdir); - }); - }); -}); diff --git a/tests/repo_tests/valid_package_download.test.ts b/tests/repo_tests/valid_package_download.test.ts index b2278909e..3e8c1c31f 100644 --- a/tests/repo_tests/valid_package_download.test.ts +++ b/tests/repo_tests/valid_package_download.test.ts @@ -3,6 +3,7 @@ import path from "path"; import { REPO_ROOT } from "tests/utils"; import { parseYaml } from "tests/utils/trunk_config"; +// TODO(Tyler): Move this to 'definition-checker' linter. // These linters use go or rust downloads. const excludedLinters: string[] = [ "clippy",