diff --git a/.github/workflows/codebase-tests.yml b/.github/workflows/codebase-tests.yml new file mode 100644 index 0000000..129f16e --- /dev/null +++ b/.github/workflows/codebase-tests.yml @@ -0,0 +1,34 @@ +name: Codebase tests (Jest) + +on: + pull_request: + types: [opened, edited, reopened, synchronize] + push: + branches: [ master ] + + +jobs: + codebase-tests: + runs-on: ubuntu-latest + steps: + - name: Repository checkout + uses: actions/checkout@v4 + + - name: Setup Node.js + uses: actions/setup-node@v4 + with: + node-version: '18.x' + cache: 'npm' + + - name: Install dependencies + run: npm install --no-progress --no-update-notifier + + - name: Run tests + run: npm test + + - name: Publish Test Report + uses: actions/upload-artifact@v3 + if: always() + with: + name: junit-report + path: reports/report.xml diff --git a/.github/workflows/danger-this.yml b/.github/workflows/danger-this.yml new file mode 100644 index 0000000..173c83a --- /dev/null +++ b/.github/workflows/danger-this.yml @@ -0,0 +1,28 @@ +name: DangerJS (contributors) +on: + pull_request: + types: [opened, edited, reopened, synchronize] + +jobs: + danger: + runs-on: ubuntu-latest + steps: + - name: Repository checkout + uses: actions/checkout@v4 + + - name: Setup Node.js + uses: actions/setup-node@v4 + with: + node-version: '18.x' + cache: 'npm' + + - name: Install dependencies + run: npm install --no-progress --no-update-notifier + + - name: Run DangerJS + run: npx danger --dangerfile=src/dangerfile.ts ci + env: + GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} + CONTRIBUTING_GUIDE_FILE: 'CONTRIBUTING.md' + MAX_COMMITS: '7' + MAX_COMMITS_WARN: '9' diff --git a/.github/workflows/pre-commit.yml b/.github/workflows/pre-commit.yml new file mode 100644 index 0000000..e1ff89c --- /dev/null +++ b/.github/workflows/pre-commit.yml @@ -0,0 +1,29 @@ +name: Pre-commit hooks CI +on: + pull_request: + types: [opened, edited, reopened, synchronize] + push: + branches: [ master ] + +jobs: + pre-commit: + runs-on: ubuntu-latest + steps: + - name: Checkout code + uses: actions/checkout@v4 + + - name: Set up Python + uses: actions/setup-python@v4 + with: + python-version: '3.9' + + - name: Install pre-commit + run: pip install pre-commit + + - name: Run pre-commit on changed files + if: github.event_name == 'pull_request' + run: | + git fetch origin ${{ github.base_ref }} + git diff --name-only FETCH_HEAD $(git merge-base FETCH_HEAD ${{ github.base_ref }}) | xargs pre-commit run --files + env: + SKIP: "conventional-precommit-linter,eslint" diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 8c12a2a..326a2ac 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -1,5 +1,116 @@ # Contributing Guide +By following these guidelines, you'll contribute effectively to our project, ensuring code quality, maintainability, and seamless collaboration. + +If you want to contribute to _Shared GitHub DangerJS_ project, here's what you need to know: + +- [Contributing Guide](#contributing-guide) + - [Code and Testing](#code-and-testing) + - [Guidelines and Best Practices](#guidelines-and-best-practices) + - [Documentation and Maintenance](#documentation-and-maintenance) + - [Tools and Extensions](#tools-and-extensions) + - [Local Testing](#local-testing) + +## Code and Testing + +- **Code Style and Structure:** + + - **Pre-Commit Hooks:** Install pre-commit hooks in this repository using the `pre-commit install` command. + + - **Readable Code Structure:** Structure your code in a readable manner. The main logic should be in the default rule function, with implementation details in helper functions. Avoid nested `if` statements and unnecessary `else` statements to maintain code clarity and simplicity. + + - **Remove Debug Statements:** Remove any debug statements (e.g., `console.log`) from your rules. + + - **Handling Rule Exit Points:** All exit points from the rule must be handled by `recordRuleExitStatus` function. Expected outputs are `Passed`, `Failed`, `Passed (with suggestions)`, or `Skipped ()`. + + - **Environment Variables:** For any new rule, add all necessary environment variables to the `action.yml` file's variables section and `src/configParameters.ts`. Ensure these are properly described in the documentation. + + - **Rule Output Formatting:** If your rule produces output that is formatted in a more complicated way (e.g., nesting, output from loops), test the final output in a test project. Include a thumbnail of its appearance in your pull request (PR) for clearer understanding and review. + +- **Code Formatting and Linting:** + + - **Prettier and ESLint:** This project uses `Prettier` for code formatting and `ESLint` for linting. These checks will run as part of the pre-commit hooks and the CI pipeline. + + - **IDE Extensions:** It is recommended for developers to install `Prettier` and `ESLint` extensions in your IDE (typically VS Code) to ensure code consistency. + +- **Creating New DangerJS rules:** + + - When creating a new DangerJS rule, ensure it includes thorough tests. Reference existing rules for guidance. + + - Consider whether the new rule is universally applicable across all Espressif projects. + + - If it's specific to a single project or a small group of projects, the rule should be disabled by default. + +- **Updating Existing Danger Rules:** When updating an existing Danger rule, ensure new features or behavioral changes are covered by additional tests. + +- **Automated Tests:** Each Danger rule (e.g. `src/rulePrSize.ts`) must have an automated test in test directory (e.g. `tests/rulePrSize.test.ts`). We aim for full test coverage, so **partial tests will not be accepted**. The tests should cover all typical usage scenarios as well as edge cases to ensure robustness. + +- **Testing Tool:** We use the npm module `jest` for testing. It is recommended to run `npm test` frequently during development to ensure that all aspects of your code are functioning as expected. + +## Guidelines and Best Practices + +- **No Breaking Changes:** Since this project is used as shared CI across many projects, **breaking changes to existing code are strictly prohibited**. + +- **Release Tags:** Though the project uses release tags, most external projects that import this CI reference the `v1` branch. + +## Documentation and Maintenance + +- **Changelog:** Update the `CHANGELOG.md` when making changes to the project. + +- **Documentation:** Regularly check and update the documentation to keep it current. + + - **Rule Descriptions:** Each Danger rule should have a description in the documentation. This should include what the rule does, its usefulness, any configurable parameters, and examples of both good and bad outputs. + - **PR Descriptions and Documentation:** When contributing, describe all changes or new features in the PR (Pull Request) description as well as in the documentation. + - **Updates to Existing Rules:** When making changes to an existing rule, review the existing documentation to ensure it is up-to-date with the changes. + +## Tools and Extensions + +- **Pre-Commit Hooks:** Installing pre-commit (`pip install pre-commit`) and the project's pre-commit hooks (`pre-commit install`) is mandatory. Unformatted code will be not accepted. +- **Recommended VS Code Extensions:** These extensions can help your development of this project: + - `streetsidesoftware.code-spell-checker` + - `kisstkondoros.vscode-codemetrics` + - `Orta.vscode-jest` + - `esbenp.prettier-vscode` + - `rvest.vs-code-prettier-eslint` + +## Local Testing + +Avoid waiting for the GitLab pipeline by testing DangerJS rules locally: + +- **Environment Setup:** Rename `.env.sample` to `.env` and add the GitLab token to `DANGER_GITLAB_API_TOKEN`. Source the env to export all variables to your shell: + + ```sh + source .env + ``` + +- **Installing Dependencies:** Run `npm install` on your local machine to install project dependencies, including dev dependencies such as type checkers and the test framework. + +- **Running DangerJS Locally:** Use command to run DangerJS in your local shell, just like in the CI pipeline: + + ```sh + npx danger --dangerfile=src/dangerfile.ts pr + ``` + +- **Simplifying Local Testing:** + + - **Temporary Rule Isolation:** For faster development, you can temporarily comment out other Danger rules in `dangerfile.ts` and retain only the rule you are working on. + + - **Restoring Dangerfile:** Don’t forget to restore (uncomment) the temporarily suppressed lines in `dangerfile.ts` before pushing your changes. + + - **Running Specific Tests:** To run a specific test file only, use the command: + + ```sh + npx jest tests/.test.ts + ``` + + ... for example: + + ```sh + npx jest tests/rulePrDescription.test.ts + ``` + + - **Monitoring Output:** Pay attention to the terminal output during local testing. This unformatted Markdown output is the equivalent of the feedback message provided by the Danger bot in the actual project PR. + --- πŸ‘**Thank you for your contributions.** diff --git a/README.md b/README.md index 53dfa1d..7d79381 100644 --- a/README.md +++ b/README.md @@ -1,7 +1,140 @@ -# Shared Danger GitHub +
+ +

Espressif Shared GitHub DangerJS

+
+ + Codebase tests (Jest) +
+
-This is the DangerJS pull request linter GitHub action, that can be called from another repositories. It's purpose is to keep the style of each PR in the specified style and automatically check for simple things like correct PR description, meaningful git messages, correct PR target branch, etc. +**Welcome to the DangerJS Pull Request linter!** --TBD- --TBD- --TBD- +This is GitHub action that can be used across different Espressif GitHub repositories and helps make sure that contributors Pull Requests (PRs) follow a specific style. + +It checks things like: + +- sufficient merge request descriptions +- meaningful commit messages in conventional commit style +- size of changes in MR +- simple branch git history +- source and target branch + +Because DangerJS does this automatically, human reviewers can focus more on code changes and spend their time reviewing PRs more productively. + +--- + +- [Usage in Your Project](#usage-in-your-project) +- [DangerJS rules](#dangerjs-rules) +- [UI output](#ui-output) +- [Configuration Options](#configuration-options) +- [Project issues](#project-issues) +- [Contributing](#contributing) + +## Usage in Your Project + +To integrate DangerJS, add GitHub workflow `.github/workflows/dangerjs.yml` to your project with this content: + + +```yaml +name: DangerJS Pull Request linter +on: + pull_request_target: + types: [opened, edited, reopened, synchronize] + +permissions: + pull-requests: write + contents: write + +jobs: + pull-request-style-linter: + runs-on: ubuntu-latest + steps: + - name: Check out PR head + uses: actions/checkout@v4 + with: + ref: ${{ github.event.pull_request.head.sha }} + + - name: DangerJS pull request linter + uses: espressif/shared-github-dangerjs@v1 + env: + GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} +``` + +- **GitHub token** This token is automatically obtained from the GitHub project and its specific permissions are set in the yaml workflow file. Avoid adding unnecessarily high permissions to this token, keep it set as in the example above. + +--- + +**🀝 Quick Integration:** If your goal is to add Shared GitHub DangerJS into your project with the default Espressif settings, **then at this point you're all set!** + +For custom configurations, refer to the descriptions of individual rules to learn how to tweak them to your needs. + +--- + +## DangerJS rules + +- [Pull Request sufficient Description](docs/rules/rulePrDescription.md) +- [Pull Request Maximum Commits](docs/rules/ruleNumberOfCommits.md) +- [Pull Request Commit Messages](docs/rules/ruleCommitMessages.md) +- [Pull Request Size](docs/rules/rulePrSize.md) +- [Pull Request Source Branch name](docs/rules/ruleSourceBranchName.md) +- [Pull Request Target Branch name](docs/rules/ruleTargetBranch.md) + +## UI output + +- [BOT comment Output Instructions format](docs/rules/outputInstructions.md) + +--- + +## Configuration Options + +If your project has specific needs, Shared GitHub DangerJS can be configured to meet them. + +**Here is complete list of configurable parameters:** + +| Parameter | CI Variable | Type | Default value | +| ------------------------------------------------------ | -------------------------------------- | ---- | -------------------------------------------------- | +| Enable rule PR Description | `rule-description` | str | `"true"` (use `"false"` to disable this rule) | +| Enable rule PR Lint Commit Messages | `rule-commit-messages` | str | `"true"` (use `"false"` to disable this rule) | +| Enable rule PR Size (changed lines) | `rule-size-lines` | str | `"true"` (use `"false"` to disable this rule) | +| Enable rule PR Source branch name | `rule-source-branch` | str | `"true"` (use `"false"` to disable this rule) | +| Enable rule PR Target branch name | `rule-target-branch` | str | `"true"` (use `"false"` to disable this rule) | +| Enable rule PR Too Many Commits | `rule-max-commits` | str | `"true"` (use `"false"` to disable this check) | +| Commit message allowed "Type"s | `commit-messages-types` | str | `"change,ci,docs,feat,fix,refactor,remove,revert"` | +| Commit message maximum length "Body" line | `commit-messages-max-body-line-length` | str | `"100"` | +| Commit message maximum length "Summary" | `commit-messages-max-summary-length` | str | `"72"` | +| Commit message minimum length "Summary" | `commit-messages-min-summary-length` | str | `"20"` | +| Ignore sections of PR description when counting length | `description-ignore-sections` | str | `"related,release,breaking"` | +| Maximum changed code lines in PR | `max-size-lines` | str | `"1000"` | +| Maximum commits in PR (soft limit, throw `info`) | `max-commits-info` | str | `"2"` | +| Maximum commits in PR (hard limit, throw `warn`) | `max-commits-warn` | str | `"5"` | +| Minimum length of PR description | `description-min-length` | str | `"50"` | + +These values can be defined in your project `DangerJS Pull Request linter` workflow, for example like this: + + +```yaml + - name: DangerJS pull request linter + uses: espressif/shared-github-dangerjs@v1 + env: + GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} + with: + instructions-contributions-file: 'CONTRIBUTING.md' + instructions-cla-link: 'https://cla-assistant.io/espressif/test' + instructions-gitlab-mirror: 'true' + max-commits-info: '3' + max-commits-warn: '6' +``` + +See more config options in [DangerJS rules](#dangerjs-rules). + +--- + +## Project issues + +If you encounter any issues, feel free to report them in the [project's issues](https://github.com/espressif/shared-github-dangerjs/issues) or create Pull Request with your suggestion. + +## Contributing + +πŸ“˜ If you are interested in contributing to this project, see the [project Contributing Guide](CONTRIBUTING.md). + +--- diff --git a/action.yml b/action.yml index d9b5276..16b7948 100644 --- a/action.yml +++ b/action.yml @@ -2,12 +2,91 @@ name: 'DangerJS Action' description: 'Run DangerJS' inputs: - custom: - description: 'Custom input example' - required: true + rule-max-commits: + description: 'Enabled rule for Maximum number of commits in PR' + required: false + max-commits-info: + description: 'Maximum number of commits in PR (throw MESSAGE)' + required: false + max-commits-warn: + description: 'Maximum number of commits in PR (throw WARN)' + required: false + + rule-commit-messages: + description: 'Enabled rule for Linting commit messages in PR' + required: false + commit-messages-types: + description: 'Commit message allowed types' + required: false + commit-messages-min-summary-length: + description: 'Commit message minimum summary length' + required: false + commit-messages-max-summary-length: + description: 'Commit message maximum summary length' + required: false + commit-messages-max-body-line-length: + description: 'Commit message maximum body line length' + required: false + + rule-description: + description: 'Enabled rule for check of PR description' + required: false + description-min-length: + description: 'Minimum length of PR description' + required: false + description-ignore-sections: + description: 'Ignored sections of PR description' + required: false + + rule-source-branch: + description: 'Enabled rule for check of PR source branch name' + required: false + + rule-target-branch: + description: 'Enabled rule for check of PR target branch is default branch' + required: false + + rule-size-lines: + description: 'Enabled rule for check of PR number of changed lines' + required: false + max-size-lines: + description: 'Maximum number of changed lines in PR' + required: false + + # Output + instructions: + description: 'Enabled output instructions under issue table in the PR comment' + required: false + instructions-contributions-file: + description: 'Custom name of Contributions Guide file in repository' + required: false + instructions-gitlab-mirror: + description: 'Enabled info about this is only Gitlab mirror' + required: false + instructions-cla-link: + description: 'Link to project Contributor License Agreement' + required: false runs: using: 'docker' image: 'Dockerfile' env: - CUSTOM: ${{ inputs.custom }} + ENABLE_RULE_PR_COMMIT_MESSAGES: ${{ inputs.rule-commit-messages }} + ENABLE_RULE_PR_DESCRIPTION: ${{ inputs.rule-description }} + ENABLE_RULE_PR_SOURCE_BRANCH_NAME: ${{ inputs.rule-source-branch }} + ENABLE_RULE_PR_TARGET_BRANCH: ${{ inputs.rule-target-branch }} + ENABLE_RULE_PR_TOO_MANY_COMMITS: ${{ inputs.rule-max-commits }} + ENABLE_RULE_PR_SIZE_LINES: ${{ inputs.rule-size-lines }} + ENABLE_OUTPUT_INSTRUCTIONS: ${{ inputs.instructions }} + CLA_LINK: ${{ inputs.instructions-cla-link}} + COMMIT_MESSAGE_ALLOWED_TYPES: ${{ inputs.commit-messages-types }} + CONTRIBUTING_GUIDE_FILE: ${{ inputs.instructions-contributions-file }} + IGNORED_SECTIONS_DESCRIPTION: ${{ inputs.description-ignore-sections }} + IS_GITLAB_MIRROR: ${{ inputs.instructions-gitlab-mirror}} + MAX_COMMIT_MESSAGE_BODY_LINE: ${{ inputs.commit-messages-max-body-line-length }} + MAX_COMMIT_MESSAGE_SUMMARY: ${{ inputs.commit-messages-max-summary-length }} + MAX_COMMITS_WARN: ${{ inputs.max-commits-warn }} + MAX_COMMITS: ${{ inputs.max-commits-info }} + MAX_PR_LINES: ${{ inputs.max-size-lines }} + MIN_COMMIT_MESSAGE_SUMMARY: ${{ inputs.commit-messages-min-summary-length }} + MIN_PR_DESCRIPTION_LENGTH: ${{ inputs.description-min-length }} diff --git a/docs/diagrams/rulePrDescription.drawio.svg b/docs/diagrams/rulePrDescription.drawio.svg new file mode 100644 index 0000000..2dcdbd8 --- /dev/null +++ b/docs/diagrams/rulePrDescription.drawio.svg @@ -0,0 +1,203 @@ + + + + + + + + + +
+
+
+ PR description +
+
+
+ + + PR description + + + + + + + + + + + +
+
+
+ description exists? +
+
+
+ + + description exis... + + + + + + + +
+
+
+ no +
+
+
+ + + no + + + + + + + +
+
+
+ OK +
+
+
+ + + OK + + + + + + + +
+
+
+ WARN: +
+ Missing PR Description +
+
+
+ + + WARN:... + + + + + + + + + +
+
+
+ remove all HTML comments +
+
+
+ + + remove all HTML comments + + + + + + + + + +
+
+
+ remove ignored sections +
+
+
+ + + remove ignored sections + + + + + + + + + + + +
+
+
+ is PR +
+ description long enough? +
+
+
+ + + is PR... + + + + + + + +
+
+
+ WARN: +
+ PR Description very brief, please add more details +
+
+
+ + + WARN:... + + + + + + + +
+
+
+ no +
+
+
+ + + no + + + + + + + + + Text is not SVG - cannot display + + + + diff --git a/docs/espressif-danger.png b/docs/espressif-danger.png new file mode 100644 index 0000000..9c6d0bf Binary files /dev/null and b/docs/espressif-danger.png differ diff --git a/docs/rules/outputInstructions.md b/docs/rules/outputInstructions.md new file mode 100644 index 0000000..6429c4e --- /dev/null +++ b/docs/rules/outputInstructions.md @@ -0,0 +1,76 @@ +# BOT comment Output Instructions format + +- code: `src/outputInstructions.ts` + +This section of the documentation describes the functionality and use of a custom script designed to automate additional instructions for Pull Request (PR) authors. + +## Always shown + +- **Personalized Greeting:** Greeting the PR author, acknowledging their contribution to the project. + +- **Collapsible Instruction Sections:** Detailed instructions are offered in a collapsible format, making the comment section cleaner and more organized. + + - The **first section** covers **general instructions about the PR linter DangerJS**, its role, and its limitations. + - The **second section** provides information on the **review and merge process**, giving authors a clear understanding of what to expect. + +- **Dynamic Messages:** Messages are dynamically generated based on the PR's content and the type of issues detected by DangerJS. + +## Conditionally shown + +- **Contributions Guide Link:** If set, the output instructions contain a link to the project's Contributions Guide, offering guidance on best practices. + +- **Contributor License Agreement (CLA):** If set, A reminder to read and sign the CLA.. + +- **GitLab Mirror Project:** For projects that are only GitLab public mirror, the output instructions contain additional instructions about the synchronization and review process between GitHub and the internal Git repository. + +--- + +## Custom Configuration + +To disable additional instructions, keeping only the table with Danger issues: + + +```yaml + - name: DangerJS pull request linter + uses: espressif/shared-github-dangerjs@v1 + with: + instructions: 'false' +``` + +When the GitHub repository serves as a public mirror of an internal GitLab project, this setting extends the Review instructions in the collapsible section: + + +```yaml + - name: DangerJS pull request linter + uses: espressif/shared-github-dangerjs@v1 + with: + instructions-gitlab-mirror: 'true' +``` + +If a Contributions Guide file is specified, a reminder with the link is included in the output: + +> πŸ“˜ Please review the project's [Contributions Guide](../../CONTRIBUTING.md) for key guidelines on code ... + + +```yaml + - name: DangerJS pull request linter + uses: espressif/shared-github-dangerjs@v1 + with: + instructions-contributions-file: 'CONTRIBUTING.md' +``` + +If a Contributor License Agreement link is specified, a reminder with the link is included in the output: + +> πŸ–ŠοΈ Please also make sure you have **read and signed** the [Contributor License Agreement](https://cla-assistant.io/espressif/project1) for this project. + + +```yaml + - name: DangerJS pull request linter + uses: espressif/shared-github-dangerjs@v1 + with: + instructions-cla-link: 'https://cla-assistant.io/espressif/project1' +``` + +--- + +- [Back to README](../../README.md) diff --git a/docs/rules/ruleCommitMessages.md b/docs/rules/ruleCommitMessages.md new file mode 100644 index 0000000..65589e1 --- /dev/null +++ b/docs/rules/ruleCommitMessages.md @@ -0,0 +1,65 @@ +# Commit Messages in a Pull Request (PR) + +- code: `src/ruleCommitMessages.ts` +- rule failing output is: `warn` (⚠️ ) + +Following a consistent commit message style helps in keeping the project well-organized and makes the collaboration process smoother. + +- **What Are We Checking?** We're linting all commit messages in the PR to align with Espressif's conventional commit style, using an external npm module called `commitlint`. + +- **If There's an Issue:** If any commit messages don't meet the standards, you'll receive specific instructions on what's wrong and how to fix it. + +- **Strong Recommendation:** Add the [Conventional Precommit Linter](https://github.com/espressif/conventional-precommit-linter) tool to your project as a pre-commit hook. It alerts users about incorrect commit messages when committing, so they don't have to wait for the DangerJS check. The DangerJS check then becomes a fallback for those who don't use pre-commit. + +- **Consistency Between Checks:** Both the DangerJS check and Conventional Precommit Linter follow the same default rules. + +--- + +## Custom Configuration + +**❕ Important:** If your project uses a custom configuration for this rule, consider applying the same settings to the [Conventional Precommit Linter](https://github.com/espressif/conventional-precommit-linter) if it's part of your project's pre-commit hooks. This ensures consistency, as both Danger and the pre-commit hook will follow the same logic. + +Disable this rule: + + +```yaml + - name: DangerJS pull request linter + uses: espressif/shared-github-dangerjs@v1 + with: + rule-commit-messages: 'false' +``` + +Add `style` to allowed types of commit message: + + +```yaml + - name: DangerJS pull request linter + uses: espressif/shared-github-dangerjs@v1 + with: + commit-messages-types: 'change,ci,docs,feat,fix,refactor,remove,revert,style' +``` + +Set allowed minimum summary length between 35 and 65 characters: + + +```yaml + - name: DangerJS pull request linter + uses: espressif/shared-github-dangerjs@v1 + with: + commit-messages-min-summary-length: '35' + commit-messages-max-summary-length: '60' +``` + +Set maximum body length to 80 characters: + + +```yaml + - name: DangerJS pull request linter + uses: espressif/shared-github-dangerjs@v1 + with: + commit-messages-max-body-line-length: '80' +``` + +--- + +- [Back to README](../../README.md) diff --git a/docs/rules/ruleNumberOfCommits.md b/docs/rules/ruleNumberOfCommits.md new file mode 100644 index 0000000..d06de16 --- /dev/null +++ b/docs/rules/ruleNumberOfCommits.md @@ -0,0 +1,63 @@ +# Number of Commits in Pull Request (PR) + +- code: `src/ruleNumberOfCommits.ts` +- rule failing output is: + - `message` (πŸ“–) - if exceeded soft limit; default 2 commits + - `warn`(⚠️ ) - if exceeded hard limit; default 5 commits; catching mostly forgotten squash after development + +When creating a Pull Request (PR), it's usually best to keep the number of commits to a minimum. Here's a guideline: + +- **Aim for Fewer Commits:** Try to keep the number of commits in one PR low. Generally, more than 2 commits within one PR is not recommended. + +- **When to Split Commits:** Sometimes, splitting your changes into more commits can be helpful. But balance this with keeping the git branch history clean and simple. + +- **Why This Matters:** Having fewer commits helps make the history of changes more understandable. It keeps things organized, making it easier for everyone to follow along. + +--- + +## Custom Configuration + +Disable this rule: + + +```yaml + - name: DangerJS pull request linter + uses: espressif/shared-github-dangerjs@v1 + with: + rule-max-commits: 'false' +``` + +Set **soft limit** custom allowed number of commits in PR for 4 (if exceeded, throw `message` (πŸ“–)): + + +```yaml + - name: DangerJS pull request linter + uses: espressif/shared-github-dangerjs@v1 + with: + max-commits-info: '4' +``` + +Set **hard limit** of custom allowed number of commits in PR for 7 (if exceeded, throw `warn`(⚠️ )) : + + +```yaml + - name: DangerJS pull request linter + uses: espressif/shared-github-dangerjs@v1 + with: + max-commits-warn: '7' +``` + +Set custom **soft limit** 4 allowed commits (if exceeded, throw `message` (πŸ“–)) **and** custom **hard limit** 7 allowed commits (if exceeded, throw `warn`(⚠️ )) : + + +```yaml + - name: DangerJS pull request linter + uses: espressif/shared-github-dangerjs@v1 + with: + max-commits-info: '4' + max-commits-warn: '7' +``` + +--- + +- [Back to README](../../README.md) diff --git a/docs/rules/rulePrDescription.md b/docs/rules/rulePrDescription.md new file mode 100644 index 0000000..3f739b2 --- /dev/null +++ b/docs/rules/rulePrDescription.md @@ -0,0 +1,56 @@ +# Pull Request (PR) sufficient Description + +- code: `src/rulePrDescription.ts` +- rule failing output is: `warn` (⚠️ ) + +The description of a pull request (PR) helps others understand the changes that have been made. Here's how to create a good description. + +
+ +
+
+ +- **What's Considered as the Description?** The PR description includes all text outside of typical metadata sections like `## Related` or `## Breaking Changes` (ignored sections can be configured). HTML comments in the PR description are also ignored. For example, if sections such as `## Related` and `## Breaking Changes` are marked as ignored, their content will not count towards the description's length. + +- **Configurable Parameters:** + - Sections can be configured to be ignored when evaluating the PR description's length. Ignored sections are defined by the workflow variable `description-ignore-sections`. By default, sections starting with `## Release ...`, `## Related ...`, and `## Breaking Change ...` are ignored. + - The minimum length of the PR description can also be configured (using the workflow variable `description-min-length`); the default is 50 characters. + +--- + +## Custom Configuration + +Disable this rule: + + +```yaml + - name: DangerJS pull request linter + uses: espressif/shared-github-dangerjs@v1 + with: + rule-description: 'false' +``` + +To ignore the sections `Release notes` and `Testing`, with a minimum PR description length of 100 characters: + + +```yaml + - name: DangerJS pull request linter + uses: espressif/shared-github-dangerjs@v1 + with: + description-min-length: '100' + description-ignore-sections: 'Release notes,Testing' +``` + +Do not ignore any sections (except HTML comments): + + +```yaml + - name: DangerJS pull request linter + uses: espressif/shared-github-dangerjs@v1 + with: + description-ignore-sections: '' +``` + +--- + +- [Back to README](../../README.md) diff --git a/docs/rules/rulePrSize.md b/docs/rules/rulePrSize.md new file mode 100644 index 0000000..dae6eab --- /dev/null +++ b/docs/rules/rulePrSize.md @@ -0,0 +1,32 @@ +# Pull Request (PR) Size + +- code: `src/rulePRSize.ts` +- rule failing output is: + - `warn`(⚠️ ) - unable read PR, probably empty PR (no code changes pushed) + - `message` (πŸ“–) - too much lines in PR + +Managing the size of an PR is important for an efficient review process. That way we can be sure that the changes are carefully examined and understood. + +- **Size Limit:** The PR should ideally contain changes to fewer than 1000 lines. If the changes exceed this threshold, it's advisable to divide them into smaller PRs. + +- **Why Keep It Small?** Reviewing a very large PR can become difficult and messy quickly. Smaller PRs are easier to handle and understand. + +- **Exceptions:** There may be cases where a large change in one PR is justified, but as a rule of thumb, we generally try to avoid these. + +--- + +## Custom Configuration + +Disable this rule: + + +```yaml + - name: DangerJS pull request linter + uses: espressif/shared-github-dangerjs@v1 + with: + rule-size-lines: 'false' +``` + +--- + +- [Back to README](../../README.md) diff --git a/docs/rules/ruleSourceBranchName.md b/docs/rules/ruleSourceBranchName.md new file mode 100644 index 0000000..82f6212 --- /dev/null +++ b/docs/rules/ruleSourceBranchName.md @@ -0,0 +1,30 @@ +# Pull Request (PR) Target Branch + +- code: `src/ruleTargetBranch.ts` +- rule failing output is: `warn` (⚠️ ) + +Selecting the appropriate target branch for a pull request (PR) is important for a seamless integration process and maintaining an organized Git history. It ensures that changes are merged into the correct branch, avoiding potential conflicts or errors. + +- **Default Branch as Target:** The target branch for the pull request **must be the default branch** of the project. This ensures a consistent flow and minimizes the risk of merging changes into the wrong branch. + +- **Why This Rule?** Using the default branch as the target for PRs maintains uniformity and order in our repository. It simplifies the process for contributors and maintainers alike, making it easier to manage the project's development. + +- **Exceptions:** In certain situations, targeting a branch other than the default may be necessary. These cases should be rare and well-justified to avoid confusion and maintain the integrity of the project's codebase. + +--- + +## Custom Configuration + +To disable this rule: + + +```yaml + - name: DangerJS pull request linter + uses: espressif/shared-github-dangerjs@v1 + with: + rule-target-branch: 'false' +``` + +--- + +- [Back to README](../../README.md) diff --git a/docs/rules/ruleTargetBranch.md b/docs/rules/ruleTargetBranch.md new file mode 100644 index 0000000..c5faa12 --- /dev/null +++ b/docs/rules/ruleTargetBranch.md @@ -0,0 +1,32 @@ +# Pull Request (PR) Target Branch + +- code: `src/ruleTargetBranch.ts` +- rule failing output is: `warn` (⚠️ ) + +Properly naming the source branch is important for maintaining a clean and manageable Git history, as well as for avoiding issues with Git synchronization and case-insensitive file systems (such as macOS). + +- **Slash Limit:** The source branch name should contain no more than one slash (`/`). Multiple slashes can lead to complications, especially during Git synchronization. + +- **Case Sensitivity:** The source branch name should be entirely in lowercase. Using uppercase letters can cause issues on case-insensitive file systems, making it difficult to switch branches or causing errors during cloning. + +- **Why These Rules?** Adhering to these naming conventions ensures that our Git operations run smoothly across different operating systems and minimizes the risk of sync issues. It also makes it easier to understand the branch's purpose at a glance. + +- **Exceptions:** While these are general guidelines, there may be exceptional cases where deviations are acceptable. However, it's advisable to stick to these rules for the sake of consistency and to avoid potential issues. + +--- + +## Custom Configuration + +Disable this rule: + + +```yaml + - name: DangerJS pull request linter + uses: espressif/shared-github-dangerjs@v1 + with: + rule-source-branch: 'false' +``` + +--- + +- [Back to README](../../README.md) diff --git a/entrypoint.sh b/entrypoint.sh index ff7c271..4dda159 100755 --- a/entrypoint.sh +++ b/entrypoint.sh @@ -8,9 +8,5 @@ cp -r /src/* /github/workspace # Change to the workspace directory cd /github/workspace || exit -echo "Custom input: ${CUSTOM}" - -env - # Run DangerJS npx danger ci --failOnErrors -v diff --git a/src/configParameters.ts b/src/configParameters.ts new file mode 100644 index 0000000..a5aa0ed --- /dev/null +++ b/src/configParameters.ts @@ -0,0 +1,187 @@ +/** + * `defaults` object: + * This object defines the default settings for various merge request checks. + * Each section corresponds to a specific check and contains default values for its parameters. + * If an environment variable for a parameter is not set, the system will use the default value from this object. + */ +const defaults = { + instructions: { + enabled: true, + isGitlabMirror: false, + contributingGuideFile: '', + claLink: '', + }, + numberOfCommits: { enabled: true, maxCommitsInfo: 2, maxCommitsWarning: 5 }, + prDescription: { enabled: true, minLength: 50, ignoredSections: 'related,release,breaking' }, + commitMessages: { + enabled: true, + allowedTypes: 'change,ci,docs,feat,fix,refactor,remove,revert', + minSummaryLength: 20, + maxSummaryLength: 72, + maxBodyLineLength: 100, + }, + prSize: { enabled: true, maxChangedLines: 1000 }, + sourceBranchName: { enabled: true }, + targetBranch: { enabled: true }, +}; + +/** + * `config` object: + * This object fetches the values of environment variables and uses them to configure the merge request checks. + * If an environment variable is not set, the system will fall back to the default value from the `defaults` object. + */ +const config = { + instructions: { + enabled: getEnvBool(process.env.ENABLE_OUTPUT_INSTRUCTIONS) ?? defaults.instructions.enabled, + isGitlabMirror: getEnvBool(process.env.IS_GITLAB_MIRROR) ?? defaults.instructions.isGitlabMirror, + contributingGuideFile: process.env.CONTRIBUTING_GUIDE_FILE || defaults.instructions.contributingGuideFile, + claLink: process.env.CLA_LINK || defaults.instructions.claLink, + }, + numberOfCommits: { + enabled: getEnvBool(process.env.ENABLE_RULE_PR_TOO_MANY_COMMITS) ?? defaults.numberOfCommits.enabled, + maxCommitsInfo: Number(process.env.MAX_COMMITS) || defaults.numberOfCommits.maxCommitsInfo, + maxCommitsWarning: Number(process.env.MAX_COMMITS_WARN) || defaults.numberOfCommits.maxCommitsWarning, + }, + prDescription: { + enabled: getEnvBool(process.env.ENABLE_RULE_PR_DESCRIPTION) ?? defaults.prDescription.enabled, + minLength: Number(process.env.MIN_PR_DESCRIPTION_LENGTH) || defaults.prDescription.minLength, + ignoredSections: process.env.IGNORED_SECTIONS_DESCRIPTION || defaults.prDescription.ignoredSections, + }, + commitMessages: { + enabled: getEnvBool(process.env.ENABLE_RULE_PR_COMMIT_MESSAGES) ?? defaults.commitMessages.enabled, + allowedTypes: process.env.COMMIT_MESSAGE_ALLOWED_TYPES || defaults.commitMessages.allowedTypes, + minSummaryLength: Number(process.env.MIN_COMMIT_MESSAGE_SUMMARY) || defaults.commitMessages.minSummaryLength, + maxSummaryLength: Number(process.env.MAX_COMMIT_MESSAGE_SUMMARY) || defaults.commitMessages.maxSummaryLength, + maxBodyLineLength: Number(process.env.MAX_COMMIT_MESSAGE_BODY_LINE) || defaults.commitMessages.maxBodyLineLength, + }, + prSize: { + enabled: getEnvBool(process.env.ENABLE_RULE_PR_SIZE_LINES) ?? defaults.prSize.enabled, + maxChangedLines: Number(process.env.MAX_PR_LINES) || defaults.prSize.maxChangedLines, + }, + sourceBranchName: { + enabled: getEnvBool(process.env.ENABLE_RULE_PR_SOURCE_BRANCH_NAME) ?? defaults.sourceBranchName.enabled, + }, + targetBranch: { + enabled: getEnvBool(process.env.ENABLE_RULE_PR_TARGET_BRANCH) ?? defaults.targetBranch.enabled, + }, +}; + +/** + * `parametersForTable` array: + * This array maps environment variables to their current values and default values. + * It is used to display a table that shows which checks are active and their current configurations in CI job tracelog. + */ +const parametersForTable = [ + { ciVar: 'ENABLE_RULE_PR_COMMIT_MESSAGES', value: config.commitMessages.enabled, defaultValue: defaults.commitMessages.enabled }, + { ciVar: 'ENABLE_RULE_PR_DESCRIPTION', value: config.prDescription.enabled, defaultValue: defaults.prDescription.enabled }, + { ciVar: 'ENABLE_RULE_PR_SIZE_LINES', value: config.prSize.enabled, defaultValue: defaults.prSize.enabled }, + { ciVar: 'ENABLE_RULE_PR_SOURCE_BRANCH_NAME', value: config.sourceBranchName.enabled, defaultValue: defaults.sourceBranchName.enabled }, + { ciVar: 'ENABLE_RULE_PR_TARGET_BRANCH', value: config.targetBranch.enabled, defaultValue: defaults.targetBranch.enabled }, + { ciVar: 'ENABLE_RULE_PR_TOO_MANY_COMMITS', value: config.numberOfCommits.enabled, defaultValue: defaults.numberOfCommits.enabled }, + { ciVar: 'ENABLE_OUTPUT_INSTRUCTIONS', value: config.instructions.enabled, defaultValue: defaults.instructions.enabled }, + { ciVar: 'CLA_LINK', value: config.instructions.claLink, defaultValue: defaults.instructions.claLink }, + { ciVar: 'COMMIT_MESSAGE_ALLOWED_TYPES', value: config.commitMessages.allowedTypes, defaultValue: defaults.commitMessages.allowedTypes }, + { ciVar: 'CONTRIBUTING_GUIDE_FILE', value: config.instructions.contributingGuideFile, defaultValue: defaults.instructions.contributingGuideFile }, + { ciVar: 'IGNORED_SECTIONS_DESCRIPTION', value: config.prDescription.ignoredSections, defaultValue: defaults.prDescription.ignoredSections }, + { ciVar: 'IS_GITLAB_MIRROR', value: config.instructions.isGitlabMirror, defaultValue: defaults.instructions.isGitlabMirror }, + { ciVar: 'MAX_COMMIT_MESSAGE_BODY_LINE', value: config.commitMessages.maxBodyLineLength, defaultValue: defaults.commitMessages.maxBodyLineLength }, + { ciVar: 'MAX_COMMIT_MESSAGE_SUMMARY', value: config.commitMessages.maxSummaryLength, defaultValue: defaults.commitMessages.maxSummaryLength }, + { ciVar: 'MAX_COMMITS_WARN', value: config.numberOfCommits.maxCommitsWarning, defaultValue: defaults.numberOfCommits.maxCommitsWarning }, + { ciVar: 'MAX_COMMITS', value: config.numberOfCommits.maxCommitsInfo, defaultValue: defaults.numberOfCommits.maxCommitsInfo }, + { ciVar: 'MAX_PR_LINES', value: config.prSize.maxChangedLines, defaultValue: defaults.prSize.maxChangedLines }, + { ciVar: 'MIN_COMMIT_MESSAGE_SUMMARY', value: config.commitMessages.minSummaryLength, defaultValue: defaults.commitMessages.minSummaryLength }, + { ciVar: 'MIN_PR_DESCRIPTION_LENGTH', value: config.prDescription.minLength, defaultValue: defaults.prDescription.minLength }, +]; + +interface RuleExitStatus { + message: string; + status: string; +} +const outputStatuses: RuleExitStatus[] = []; + +/** + * Logs the status of a rule with padded formatting and stores it in the `outputStatuses` array. + * If the rule already exists in the array, its status is updated. + */ +function recordRuleExitStatus(message: string, status: string) { + // Check if the rule already exists in the array + const existingRecord = outputStatuses.find((rule) => rule.message === message); + + // Update or create the status of the existing rule in array + if (existingRecord) existingRecord.status = status; + else outputStatuses.push({ message, status }); +} + +/** + * Displays all the rule output statuses stored in the `outputStatuses` array. + * Filters out any empty lines, sorts them alphabetically, and prints the statuses + * with a header and separator. + * These statuses are later displayed in CI job tracelog. + */ +function displayAllOutputStatuses() { + const lineLength = 100; + const sortedStatuses = outputStatuses.sort((a, b) => a.message.localeCompare(b.message)); + + const formattedLines: string[] = sortedStatuses.map((statusObj) => { + const paddingLength = lineLength - statusObj.message.length - statusObj.status.length; + const paddedMessage = statusObj.message.padEnd(statusObj.message.length + paddingLength, '.'); + return `${paddedMessage} ${colorize(statusObj.status)}`; + }); + + console.log('DangerJS checks (rules) output states:\n' + '='.repeat(lineLength + 2)); + console.log(formattedLines.join('\n')); + console.log('='.repeat(lineLength + 2)); +} + +/** + * This function logs a table to the console, displaying the current configuration of merge request Danger checks. + * The table shows the environment variable associated with each check, its current value, and whether it's using + * the default setting or a custom one. + */ +function logParamTable() { + interface JobParameter { + CiVariable: string; + Value: number | string | boolean; + CustomSettings: string; + } + + const jobParametersTable: JobParameter[] = parametersForTable.map((param) => ({ + CiVariable: param.ciVar, + Value: param.value, + CustomSettings: param.value == param.defaultValue ? 'default' : `custom (default is: ${param.defaultValue})`, + })); + + console.table(jobParametersTable); +} + +/** + * This function converts environment variable values to a boolean representation. + * The function checks if the provided value matches any of the common representations of "true" + * such as 'true', 'on', 'yes', '1', and 'enabled'. The check is case-insensitive. + * If the value matches any of these representations, the function returns boolean `true`; otherwise, it returns boolean `false`. + * If the value is undefined, the function returns undefined; that results to using default value in config object + */ +function getEnvBool(value: string | undefined): boolean | undefined { + if (!value) return undefined; // Checks for both undefined and empty string + return ['true', 'on', 'yes', '1', 'enabled'].includes(value.toLowerCase()); +} + +/** + * This function applies specific ANSI color codes to a status message string based on its content, + * enhancing the readability of console/CI job tracelog outputs. It checks the content of the message and colors it + * accordingly: 'passed' messages are green, 'failed' messages are red, and all other cases are yellow. + */ +function colorize(message: string): string { + enum AnsiColors { + RED = '\u001b[31m', + GREEN = '\u001b[32m', + YELLOW = '\u001b[33m', + RESET = '\u001b[0m', + } + if (message.toLowerCase().startsWith('skipped')) return message; + if (message.toLowerCase() === 'passed') return AnsiColors.GREEN + message + AnsiColors.RESET; + if (message.toLowerCase() === 'failed') return AnsiColors.RED + message + AnsiColors.RESET; + return AnsiColors.YELLOW + message + AnsiColors.RESET; +} + +export { displayAllOutputStatuses, recordRuleExitStatus, outputStatuses, logParamTable, config }; diff --git a/src/dangerfile.ts b/src/dangerfile.ts index fdc14a8..b6139fb 100644 --- a/src/dangerfile.ts +++ b/src/dangerfile.ts @@ -1,24 +1,37 @@ -import { DangerResults, DangerDSLType } from 'danger'; +import { DangerResults } from 'danger'; +import { config, displayAllOutputStatuses, logParamTable } from './configParameters'; +import ruleCommitMessages from './ruleCommitMessages'; +import rulePrSize from './rulePrSize'; +import ruleNumberOfCommits from './ruleNumberOfCommits'; +import rulePrDescription from './rulePrDescription'; +import ruleSourceBranchName from './ruleSourceBranchName'; +import ruleTargetBranch from './ruleTargetBranch'; +import outputInstructions from './outputInstructions'; + declare const results: DangerResults; -declare const danger: DangerDSLType; declare const message: (message: string, results?: DangerResults) => void; -declare const markdown: (message: string, results?: DangerResults) => void; -declare const warn: (message: string, results?: DangerResults) => void; -function runDangerRules(): void { - console.log('Running Danger Rules ...'); +async function runDangerRules(): Promise { + // Show Danger CI job parameters - visible only in CI job trace log + logParamTable(); - message('Hello from Danger JS - message'); - markdown('Hello from Danger JS - markdown'); - warn('Hello from Danger JS - warn'); + // Run DangerJS checks if they are enabled by CI job parameters + if (config.commitMessages.enabled) await ruleCommitMessages(); + if (config.numberOfCommits.enabled) ruleNumberOfCommits(); + if (config.prDescription.enabled) rulePrDescription(); + if (config.prSize.enabled) rulePrSize(); + if (config.sourceBranchName.enabled) ruleSourceBranchName(); + if (config.targetBranch.enabled) await ruleTargetBranch(); - const prCommits = danger.git.commits; - console.log('PR Commits: ', prCommits); + // Show DangerJS individual checks statuses - visible in CI job tracelog + displayAllOutputStatuses(); - const customInput: number = Number(process.env.CUSTOM); - console.log(`CUSTOM INPUT: ${customInput}`); + // Show DangerJS dynamic output message and instructions - visible in feedback comment + await outputInstructions(); - console.log('Danger Rules results: ', results); + // Show success message if no issues are found + const foundIssues: boolean = Boolean(results.fails.length + results.warnings.length + results.messages.length); + if (!foundIssues) return message('πŸŽ‰ Good Job! All checks are passing!'); } runDangerRules(); diff --git a/src/outputInstructions.ts b/src/outputInstructions.ts new file mode 100644 index 0000000..5895900 --- /dev/null +++ b/src/outputInstructions.ts @@ -0,0 +1,103 @@ +import { DangerDSLType, DangerResults } from 'danger'; +import { Octokit } from '@octokit/rest'; +import { config } from './configParameters'; + +declare const danger: DangerDSLType; +declare const results: DangerResults; +declare const markdown: (message: string, results?: DangerResults) => void; + +/** + * Generates a greeting message and a set of instructions for the author of a Pull Request (PR). + * + * This function creates a custom message for the MR author, providing guidance on how to handle + * the automated feedback from DangerJS. + * It includes a set of recommended actions for resolving warnings and information messages, when issues are found, + * and instructions on how to retry DangerJS checks if necessary. + * Message is dynamically generated based on the type of Danger issues found in the MR. + */ +const prAuthorUsername = danger.github.pr.user.login; +const repositoryOwner = danger.github.pr.base.repo.owner.login; +const repositoryName = danger.github.pr.base.repo.name; +const dangerProjectUrl: string = 'https://github.com/espressif/shared-github-dangerjs'; + +export default async function (): Promise { + // Basic instructions (ALWAYS SHOWN) + let instructions: string = ''; + instructions += `πŸ‘‹ Hello ${prAuthorUsername}, we appreciate your contribution to this project!
`; + + // Contributors guide link, if exists in the repository + if (config.instructions.contributingGuideFile) { + const defaultBranch = await getDefaultBranch(); + const contributionsGuideLink = `https://github.com/${repositoryOwner}/${repositoryName}/blob/${defaultBranch}/${config.instructions.contributingGuideFile}`; + instructions += `
`; + instructions += `πŸ“˜ Please review the project's Contributions Guide for key guidelines on code, documentation, testing, and more.
`; + } + + // Contributor License Agreement, if provided link to it + if (config.instructions.claLink) { + instructions += `
`; + instructions += `πŸ–ŠοΈ Please also make sure you have read and signed the Contributor License Agreement for this project.
`; + } + + // Basic instructions (ALWAYS SHOWN) + instructions += `
`; + instructions += `
Click to see more instructions ...

`; // START collapsible section INSTRUCTIONS + instructions += `
This automated output is generated by the PR linter DangerJS, which checks if your Pull Request meets the project's requirements and helps you fix potential issues.

`; + instructions += `DangerJS is triggered with each \`push\` event to a Pull Request and modify the contents of this comment.

`; + instructions += `Please consider the following:
`; + instructions += `- Danger mainly focuses on the PR structure and formatting and can't understand the meaning behind your code or changes.
`; + instructions += `- Danger is not a substitute for human code reviews; it's still important to request a code review from your colleagues.
`; + + // If 'warning' or 'error' issues exist, add this to Instructions DangerJS + if (results.fails.length + results.warnings.length) { + instructions += '- Resolve all warnings (⚠️ ) before requesting a review from human reviewers - they will appreciate it.
'; + } + + // If info issues exist, add this to Instructions DangerJS + if (results.messages.length) { + instructions += `- Addressing info messages (πŸ“–) is strongly recommended; they're less critical but valuable.
`; + } + + // Add (always) retry link as last line of Instructions DangerJS + const retryLinkUrl: string = `https://github.com/${repositoryOwner}/${repositoryName}/actions`; + instructions += `- To manually retry these Danger checks, please navigate to the Actions tab and re-run last Danger workflow.
`; + instructions += `

`; // END collapsible section INSTRUCTIONS + + // Instructions about pull request Review and Merge process + instructions += `
Review and merge process you can expect ...

`; // START collapsible section REVIEW PROCESS + instructions += `
`; + if (config.instructions.isGitlabMirror) { + instructions += `We do welcome contributions in the form of bug reports, feature requests and pull requests via this public GitHub repository.

`; + instructions += `This GitHub project is public mirror of our internal git repository

`; + instructions += `1. An internal issue has been created for the PR, we assign it to the relevant engineer.
`; + instructions += `2. They review the PR and either approve it or ask you for changes or clarifications.
`; + instructions += `3. Once the GitHub PR is approved, we synchronize it into our internal git repository.
`; + instructions += `4. In the internal git repository we do the final review, collect approvals from core owners and make sure all the automated tests are passing.
`; + instructions += ` - At this point we may do some adjustments to the proposed change, or extend it by adding tests or documentation.
`; + instructions += `5. If the change is approved and passes the tests it is merged into the default branch.
`; + instructions += `5. On next sync from the internal git repository merged change will appear in this public GitHub repository.
`; + } else { + instructions += `We do welcome contributions in the form of bug reports, feature requests and pull requests.

`; + instructions += `1. An internal issue has been created for the PR, we assign it to the relevant engineer.
`; + instructions += `2. They review the PR and either approve it or ask you for changes or clarifications.
`; + instructions += `3. Once the GitHub PR is approved we do the final review, collect approvals from core owners and make sure all the automated tests are passing.
`; + instructions += ` - At this point we may do some adjustments to the proposed change, or extend it by adding tests or documentation.
`; + instructions += `4. If the change is approved and passes the tests it is merged into the default branch.
`; + } + instructions += `

`; // END collapsible section REVIEW PROCESS + instructions += `\n`; + + // Create output message + return markdown(instructions); +} + +/** + * Fetches the default branch of the repository. + */ +async function getDefaultBranch(): Promise { + const repositoryOwner: string = danger.github.pr.base.repo.owner.login; + const repositoryName: string = danger.github.pr.base.repo.name; + const octokit = new Octokit(); + const { data: repo } = await octokit.repos.get({ owner: repositoryOwner, repo: repositoryName }); + return repo.default_branch; +} diff --git a/src/ruleCommitMessages.ts b/src/ruleCommitMessages.ts new file mode 100644 index 0000000..ddf0d9d --- /dev/null +++ b/src/ruleCommitMessages.ts @@ -0,0 +1,140 @@ +import { DangerDSLType } from 'danger'; +import lint from '@commitlint/lint'; +import { QualifiedRules } from '@commitlint/types'; +import { LintRuleOutcome } from '@commitlint/types'; +import { config, recordRuleExitStatus } from './configParameters'; + +declare const danger: DangerDSLType; +declare const warn: (message: string) => void; + +const lintingRules: QualifiedRules = { + // rule definition: [(0-1 = off/on), (always/never = must be/mustn't be), (value)] + 'body-max-line-length': [1, 'always', config.commitMessages.maxBodyLineLength], // Max length of the body line + 'footer-leading-blank': [1, 'always'], // Always have a blank line before the footer section + 'footer-max-line-length': [1, 'always', config.commitMessages.maxBodyLineLength], // Max length of the footer line + 'subject-max-length': [1, 'always', config.commitMessages.maxSummaryLength], // Max length of the "Summary" + 'subject-min-length': [1, 'always', config.commitMessages.minSummaryLength], // Min length of the "Summary" + 'scope-case': [1, 'always', 'lower-case'], // "scope/component" must be lowercase + 'subject-full-stop': [1, 'never', '.'], // "Summary" must not end with a full stop (period) + 'subject-empty': [1, 'never'], // "Summary" is mandatory + 'type-case': [1, 'always', 'lower-case'], // "type/action" must start with lower-case + 'type-empty': [1, 'never'], // "type/action" is mandatory + 'type-enum': [1, 'always', config.commitMessages.allowedTypes.split(',')], // "type/action" must be one of the allowed types + 'body-leading-blank': [1, 'always'], // Always have a blank line before the body section +}; + +interface Commit { + message: string; +} + +/** + * Throw Danger WARN if commit messages are not valid based on Espressif rules for git commit messages + */ +export default async function (): Promise { + const ruleName = 'Commit messages style'; + const prCommits: Commit[] = danger.git.commits; + + const issuesAllCommitMessages: string[] = []; + + for (const commit of prCommits) { + const commitMessage: string = commit.message; + const commitMessageTitle: string = commit.message.split('\n')[0]; + + const issuesSingleCommitMessage: string[] = []; + let reportSingleCommitMessage = ''; + + // Check if the commit message contains any Jira ticket references + const jiraTicketRegex = /[A-Z0-9]+-[0-9]+/g; + const jiraTicketMatches = commitMessage.match(jiraTicketRegex); + if (jiraTicketMatches) { + const jiraTicketNames = jiraTicketMatches.join(', '); + issuesSingleCommitMessage.push( + `- probably contains Jira ticket reference (\`${jiraTicketNames}\`). Please remove Jira tickets from commit messages.`, + ); + } + + // Add check for spaces in scope + const scopeRegex = /(?<=\().+?(?=\):)/; // This regex captures the scope after the type and before the summary, including spaces + const matchResults = commitMessageTitle.match(scopeRegex); + if (matchResults !== null) { + const scope = matchResults[0]; + if (scope && scope.trim().includes(' ')) { + issuesSingleCommitMessage.push( + '- *scope/component* should be lowercase without whitespace, allowed special characters are `_` `/` `.` `,` `*` `-` `.`', + ); + } + } + + // Lint commit messages with @commitlint (Conventional Commits style) + const result = await lint(commit.message, lintingRules); + + for (const warning of result.warnings as LintRuleOutcome[]) { + // Custom messages for each rule with terminology used by Espressif conventional commits guide + switch (warning.name) { + case 'subject-max-length': + issuesSingleCommitMessage.push(`- *summary* appears to be too long`); + break; + case 'type-empty': + issuesSingleCommitMessage.push(`- *type/action* looks empty`); + break; + case 'type-case': + issuesSingleCommitMessage.push(`- *type/action* should start with a lowercase letter`); + break; + case 'scope-case': + issuesSingleCommitMessage.push( + `- *scope/component* should be lowercase without whitespace, allowed special characters are \`_\` \`/\` \`.\` \`,\` \`*\` \`-\` \`.\``, + ); + break; + case 'subject-empty': + issuesSingleCommitMessage.push(`- *summary* looks empty`); + break; + case 'subject-min-length': + issuesSingleCommitMessage.push(`- *summary* looks too short`); + break; + case 'subject-full-stop': + issuesSingleCommitMessage.push(`- *summary* should not end with a period (full stop)`); + break; + case 'type-enum': + issuesSingleCommitMessage.push( + `- *type/action* should be one of [${config.commitMessages.allowedTypes + .split(',') + .map((type) => `\`${type}\``) + .join(', ')}]`, + ); + break; + + default: + issuesSingleCommitMessage.push(`- ${warning.message}`); + } + } + + if (issuesSingleCommitMessage.length) { + reportSingleCommitMessage = `- the commit message \`"${commitMessageTitle}"\`:\n${issuesSingleCommitMessage + .map((message) => ` ${message}`) // Indent each issue by 2 spaces + .join('\n')}`; + issuesAllCommitMessages.push(reportSingleCommitMessage); + } + } + + // Create report + if (issuesAllCommitMessages.length) { + issuesAllCommitMessages.sort(); + const basicTips = [ + `- follow [Conventional Commits style](https://www.conventionalcommits.org/en/v1.0.0/)`, + `- correct format of commit message should be: \`(): \`, for example \`fix(esp32): Fixed startup timeout issue\``, + `- allowed types are: \`${config.commitMessages.allowedTypes}\``, + `- sufficiently descriptive message summary should be between ${config.commitMessages.minSummaryLength} to ${config.commitMessages.maxSummaryLength} characters and start with upper case letter`, + `- avoid Jira references in commit messages (unavailable/irrelevant for our customers)`, + ]; + const dangerMessage = `\n**Some issues found for the commit messages in this PR:**\n${issuesAllCommitMessages.join('\n')} + \n*** + \n**Please fix these commit messages** - here are some basic tips:\n${basicTips.join('\n')} + \n \`TIP:\` Install pre-commit hooks and run this check when committing (uses the [Conventional Precommit Linter](https://github.com/espressif/conventional-precommit-linter)). + `; + warn(dangerMessage); + return recordRuleExitStatus(ruleName, 'Failed'); + } + + // At this point, the rule has passed + return recordRuleExitStatus(ruleName, 'Passed'); +} diff --git a/src/ruleNumberOfCommits.ts b/src/ruleNumberOfCommits.ts new file mode 100644 index 0000000..7c87892 --- /dev/null +++ b/src/ruleNumberOfCommits.ts @@ -0,0 +1,28 @@ +import { DangerDSLType, DangerResults } from 'danger'; +import { config, recordRuleExitStatus } from './configParameters'; + +declare const danger: DangerDSLType; +declare const message: (message: string, results?: DangerResults) => void; +declare const warn: (message: string, results?: DangerResults) => void; + +/** + * Throw a Danger MESSAGE if the pull request has an excessive number of commits (if it needs to be squashed). + * Throw a Danger WARN if number of commits hits warn defined value. + */ +export default function (): void { + const ruleName = 'Number of commits in Pull Request'; + const prCommits: number = danger.github.commits.length; + + if (prCommits > config.numberOfCommits.maxCommitsWarning) { + warn(`Please consider squashing your ${prCommits} commits (simplifying branch history).`); + return recordRuleExitStatus(ruleName, 'Failed'); + } + + if (prCommits > config.numberOfCommits.maxCommitsInfo) { + message(`You might consider squashing your ${prCommits} commits (simplifying branch history).`); + return recordRuleExitStatus(ruleName, 'Passed (with suggestion)'); + } + + // At this point, the rule has passed + return recordRuleExitStatus(ruleName, 'Passed'); +} diff --git a/src/rulePrDescription.ts b/src/rulePrDescription.ts new file mode 100644 index 0000000..9cdf8b5 --- /dev/null +++ b/src/rulePrDescription.ts @@ -0,0 +1,52 @@ +/** + * Throw Danger WARN if the pull request description is not sufficiently descriptive + */ + +import { DangerDSLType, DangerResults } from 'danger'; +import { config, recordRuleExitStatus } from './configParameters'; +declare const danger: DangerDSLType; +declare const warn: (message: string, results?: DangerResults) => void; + +export default function (): void { + const ruleName = 'Pull Request sufficient Description'; + + // Check if the MR description is missing + if (!prDescriptionExists()) { + warn('The Pull Request description is empty. Please provide a detailed description.'); + return recordRuleExitStatus(ruleName, 'Failed'); + } + + let prDescription: string = danger.github.pr.body; + + // Do not count HTML comments as part of MR description length + prDescription = removeHtmlComments(prDescription); + + // Do not count specified sections as part of MR description length + const ignoredSections = config.prDescription.ignoredSections.split(','); + prDescription = removeSections(prDescription, ignoredSections); + + // Count the length of the MR description without the ignored sections and HTML comments + if (prDescription.length < config.prDescription.minLength) { + warn('The Pull Request description looks very brief, please check if more details can be added.'); + return recordRuleExitStatus(ruleName, 'Failed'); + } + + // At this point, the rule has passed + return recordRuleExitStatus(ruleName, 'Passed'); +} + +function prDescriptionExists(): boolean { + return Boolean(danger.github.pr.body?.trim()); +} + +function removeHtmlComments(description: string): string { + return description.replace(//g, ''); +} + +function removeSections(description: string, sectionNames: string[]): string { + for (const sectionName of sectionNames) { + const regexPattern = new RegExp(`## ${sectionName}.*?(\\r\\n|\\n|\\r)([\\s\\S]*?)(?=## |$)`, 'i'); + description = description.replace(regexPattern, ''); + } + return description; +} diff --git a/src/rulePrSize.ts b/src/rulePrSize.ts new file mode 100644 index 0000000..ba02472 --- /dev/null +++ b/src/rulePrSize.ts @@ -0,0 +1,22 @@ +/** + * Throw Danger MESSAGE if the pull request is too large (more than 1000 lines of changes) + * Throw Danger WARN if it is not possible to calculate PR size (empty PR) + */ + +import { DangerDSLType, DangerResults } from 'danger'; +import { config, recordRuleExitStatus } from './configParameters'; +declare const danger: DangerDSLType; +declare const message: (message: string, results?: DangerResults) => void; + +export default function (): void { + const ruleName = 'Pull Request size (number of changed lines)'; + const totalLines: number | null = danger.github.pr.additions + danger.github.pr.deletions; + + if (totalLines > config.prSize.maxChangedLines) { + message(`This PR seems to be quite large (total lines of code: ${totalLines}), you might consider splitting it into smaller PRs`); + return recordRuleExitStatus(ruleName, 'Passed (with suggestions)'); + } + + // At this point, the rule has passed + recordRuleExitStatus(ruleName, 'Passed'); +} diff --git a/src/ruleSourceBranchName.ts b/src/ruleSourceBranchName.ts new file mode 100644 index 0000000..7573963 --- /dev/null +++ b/src/ruleSourceBranchName.ts @@ -0,0 +1,39 @@ +import { DangerDSLType, DangerResults } from 'danger'; +import { recordRuleExitStatus } from './configParameters'; + +declare const danger: DangerDSLType; +declare const warn: (message: string, results?: DangerResults) => void; + +/** + * Throw Danger WARN if branch name contains more than one slash or uppercase letters + */ +export default function (): void { + const ruleName = 'Source branch name'; + + const sourceBranch = danger.github.pr.head.ref; + const issuesBranchName: string[] = []; + + // Check if the source branch name contains more than one slash + const slashCount = (sourceBranch.match(/\//g) || []).length; + + if (slashCount > 1) { + issuesBranchName.push(`- contains more than one slash (\`/\`). This can cause troubles with git sync.`); + } + + // Check if the source branch name contains any uppercase letters + if (sourceBranch !== sourceBranch.toLowerCase()) { + issuesBranchName.push(`- contains uppercase letters. This can cause troubles on case-insensitive file systems (macOS).`); + } + + // Create report + if (issuesBranchName.length) { + const warnOutput = `The source branch \`"${sourceBranch}"\` incorrect format:\n${issuesBranchName + .map((message) => ` ${message}`) // Indent each issue by 2 spaces + .join('\n')}\nPlease rename your branch.`; + warn(warnOutput); + return recordRuleExitStatus(ruleName, 'Failed'); + } + + // At this point, the rule has passed + return recordRuleExitStatus(ruleName, 'Passed'); +} diff --git a/src/ruleTargetBranch.ts b/src/ruleTargetBranch.ts new file mode 100644 index 0000000..3917a0a --- /dev/null +++ b/src/ruleTargetBranch.ts @@ -0,0 +1,39 @@ +import { DangerDSLType, DangerResults } from 'danger'; +import { Octokit } from '@octokit/rest'; +import { recordRuleExitStatus } from './configParameters'; + +declare const danger: DangerDSLType; +declare const warn: (message: string, results?: DangerResults) => void; + +/** + * Check if the target branch is project default branch. + * + * @dangerjs warn + */ +export default async function (): Promise { + const ruleName = 'Target branch is project default branch'; + const prTargetBranch: string = danger.github.pr.base.ref; + const defaultBranch: string = await getDefaultBranch(); + + if (prTargetBranch !== defaultBranch) { + warn(` + The **target branch** for this Pull Request **must be the default branch** of the project (\`${defaultBranch}\`).\n + If you would like to add this feature to a different branch, please state this in the PR description and we will consider it. + `); + return recordRuleExitStatus(ruleName, 'Failed'); + } + + // At this point, the rule has passed + return recordRuleExitStatus(ruleName, 'Passed'); +} + +/** + * Fetches the default branch of the repository. + */ +async function getDefaultBranch(): Promise { + const repoOwner: string = danger.github.pr.base.repo.owner.login; + const repoName: string = danger.github.pr.base.repo.name; + const octokit = new Octokit(); + const { data: repo } = await octokit.repos.get({ owner: repoOwner, repo: repoName }); + return repo.default_branch; +} diff --git a/tests/outputInstructions.test.ts b/tests/outputInstructions.test.ts new file mode 100644 index 0000000..92126e0 --- /dev/null +++ b/tests/outputInstructions.test.ts @@ -0,0 +1,249 @@ +import { config as originalConfig } from '../src/configParameters'; + +declare global { + var danger: any; + var results: any; + var markdown: any; +} + +function expectBasicInstructions() { + expect(markdown).toHaveBeenCalledWith(expect.stringContaining('we appreciate your contribution to this project!')); + expect(markdown).toHaveBeenCalledWith(expect.stringContaining('This automated output is generated')); + expect(markdown).toHaveBeenCalledWith(expect.stringContaining('DangerJS is triggered with each `push`')); + expect(markdown).toHaveBeenCalledWith(expect.stringContaining('Please consider the following:')); + expect(markdown).toHaveBeenCalledWith(expect.stringContaining('- Danger mainly focuses on the PR structure and formatting')); + expect(markdown).toHaveBeenCalledWith(expect.stringContaining('not a substitute for human code reviews')); + expect(markdown).toHaveBeenCalledWith(expect.stringContaining('retry these Danger checks')); +} + +function expectContributionsGuideLink(include: boolean = true) { + if (include) expect(markdown).toHaveBeenCalledWith(expect.stringContaining('Contributions Guide')); + if (!include) expect(markdown).not.toHaveBeenCalledWith(expect.stringContaining('Contributions Guide')); +} + +function expectResolveWarnings(include: boolean = true) { + if (include) expect(markdown).toHaveBeenCalledWith(expect.stringContaining('Resolve all warnings (⚠️ )')); + if (!include) expect(markdown).not.toHaveBeenCalledWith(expect.stringContaining('Resolve all warnings (⚠️ )')); +} +function expectResolveInfos(include: boolean = true) { + if (include) expect(markdown).toHaveBeenCalledWith(expect.stringContaining('Addressing info messages (πŸ“–)')); + if (!include) expect(markdown).not.toHaveBeenCalledWith(expect.stringContaining('Addressing info messages (πŸ“–)')); +} +function expectClaLink(include: boolean = true) { + if (include) expect(markdown).toHaveBeenCalledWith(expect.stringContaining('read and signed')); + if (!include) expect(markdown).not.toHaveBeenCalledWith(expect.stringContaining('read and signed')); +} +function expectGitlabMirror(include: boolean = true) { + if (include) expect(markdown).toHaveBeenCalledWith(expect.stringContaining('This GitHub project is public mirror')); + if (!include) expect(markdown).not.toHaveBeenCalledWith(expect.stringContaining('This GitHub project is public mirror')); +} + +describe('TESTS COMPONENT: outputInstructions for GitHub PRs', () => { + let outputInstructions: any; + beforeEach(async () => { + global.danger = { + github: { + pr: { + user: { login: 'JaneDoe' }, // author of Pull Request + base: { + repo: { + owner: { login: 'espressif' }, + name: 'example-repo', + default_branch: 'main', + }, + }, + }, + }, + }; + global.results = { + fails: [], + warnings: [], + messages: [], + }; + global.markdown = jest.fn(); + jest.resetModules(); + jest.mock('@octokit/rest', () => { + return { + Octokit: jest.fn().mockImplementation(() => ({ + // Mock the default branch fetch + repos: { get: jest.fn().mockResolvedValue({ data: { default_branch: 'main' } }) }, + })), + }; + }); + }); + + describe('DEFAULT: GitHub project, no CLA, no Contributions Guide', () => { + beforeEach(async () => { + jest.doMock('../src/configParameters', () => ({ + config: { + ...originalConfig, + }, + })); + outputInstructions = (await import('../src/outputInstructions')).default; + }); + + it('EXPECT PASS: No issues, only basic instructions (always there)', async () => { + await outputInstructions(); + expectBasicInstructions(); + expectGitlabMirror(false); + expectClaLink(false); + expectContributionsGuideLink(false); + expectResolveWarnings(false); + expectResolveInfos(false); + }); + + it('EXPECT FAIL: WARN issues in PR', async () => { + global.results.warnings = [{ message: 'Sample warning' }]; + await outputInstructions(); + expectBasicInstructions(); + expectGitlabMirror(false); + expectClaLink(false); + expectContributionsGuideLink(false); + expectResolveWarnings(); + expectResolveInfos(false); + }); + + it('EXPECT FAIL: ERROR issues in PR', async () => { + global.results.fails = [{ message: 'Sample error' }]; + await outputInstructions(); + expectBasicInstructions(); + expectGitlabMirror(false); + expectClaLink(false); + expectContributionsGuideLink(false); + expectResolveWarnings(); + expectResolveInfos(false); + }); + + it('EXPECT FAIL: MESSAGE (info) issues in PR', async () => { + global.results.messages = [{ message: 'Sample info message' }]; + await outputInstructions(); + expectBasicInstructions(); + expectGitlabMirror(false); + expectClaLink(false); + expectContributionsGuideLink(false); + expectResolveInfos(); + expectResolveWarnings(false); + }); + + it('EXPECT FAIL: WARN and MESSAGE (info) issues in PR', async () => { + global.results.warnings = [{ message: 'Sample warning' }]; + global.results.messages = [{ message: 'Sample info message' }]; + await outputInstructions(); + expectBasicInstructions(); + expectGitlabMirror(false); + expectClaLink(false); + expectContributionsGuideLink(false); + expectResolveInfos(); + expectResolveWarnings(); + }); + }); + describe('CUSTOM: CLA link defined', () => { + beforeEach(async () => { + jest.doMock('../src/configParameters', () => ({ + config: { + ...originalConfig, + instructions: { + claLink: 'https://cla-assistant.io/espressif/esp-idf', + }, + }, + })); + outputInstructions = (await import('../src/outputInstructions')).default; + }); + + it('EXPECT PASS: No issues, only basic instructions (always there)', async () => { + await outputInstructions(); + expectBasicInstructions(); + expectClaLink(); + expectGitlabMirror(false); + expectContributionsGuideLink(false); + expectResolveWarnings(false); + expectResolveInfos(false); + }); + + it('EXPECT FAIL: WARN and MESSAGE (info) issues in PR', async () => { + global.results.warnings = [{ message: 'Sample warning' }]; + global.results.messages = [{ message: 'Sample info message' }]; + await outputInstructions(); + expectBasicInstructions(); + expectClaLink(); + expectGitlabMirror(false); + expectContributionsGuideLink(false); + expectResolveInfos(); + expectResolveWarnings(); + }); + }); + describe('CUSTOM: Contributions Guide defined', () => { + beforeEach(async () => { + jest.doMock('../src/configParameters', () => ({ + config: { + ...originalConfig, + instructions: { + contributingGuideFile: 'CONTRIBUTING.rst', + }, + }, + })); + outputInstructions = (await import('../src/outputInstructions')).default; + }); + + it('EXPECT PASS: No issues, only basic instructions (always there)', async () => { + await outputInstructions(); + expectBasicInstructions(); + expectContributionsGuideLink(); + expect(markdown).toHaveBeenCalledWith(expect.stringContaining('main/CONTRIBUTING.rst')); + expectClaLink(false); + expectGitlabMirror(false); + expectResolveWarnings(false); + expectResolveInfos(false); + }); + + it('EXPECT FAIL: WARN and MESSAGE (info) issues in PR', async () => { + global.results.warnings = [{ message: 'Sample warning' }]; + global.results.messages = [{ message: 'Sample info message' }]; + await outputInstructions(); + expectBasicInstructions(); + expectContributionsGuideLink(); + expect(markdown).toHaveBeenCalledWith(expect.stringContaining('main/CONTRIBUTING.rst')); + expectClaLink(false); + expectGitlabMirror(false); + expectResolveInfos(); + expectResolveWarnings(); + }); + }); + describe('CUSTOM: GitHub project is Gitlab mirror', () => { + beforeEach(async () => { + jest.doMock('../src/configParameters', () => ({ + config: { + ...originalConfig, + instructions: { + isGitlabMirror: true, + }, + }, + })); + outputInstructions = (await import('../src/outputInstructions')).default; + }); + + it('EXPECT PASS: No issues, only basic instructions (always there)', async () => { + await outputInstructions(); + expectBasicInstructions(); + expectGitlabMirror(); + expectClaLink(false); + expectContributionsGuideLink(false); + expectResolveWarnings(false); + expectResolveInfos(false); + }); + + it('EXPECT FAIL: WARN and MESSAGE (info) issues in PR', async () => { + global.results.warnings = [{ message: 'Sample warning' }]; + global.results.messages = [{ message: 'Sample info message' }]; + await outputInstructions(); + expectBasicInstructions(); + expectGitlabMirror(); + expectClaLink(false); + expectContributionsGuideLink(false); + expectResolveInfos(); + expectResolveWarnings(); + }); + }); +}); + +export {}; diff --git a/tests/ruleCommitMessages.test.ts b/tests/ruleCommitMessages.test.ts new file mode 100644 index 0000000..5437422 --- /dev/null +++ b/tests/ruleCommitMessages.test.ts @@ -0,0 +1,253 @@ +import { config as originalConfig, recordRuleExitStatus } from '../src/configParameters'; + +declare global { + var danger: any; + var warn: any; +} + +describe('TESTS: Commit messages style', () => { + let ruleCommitMessages: any; + beforeEach(() => { + global.danger = { + git: { + commits: [{ message: '' }], + }, + }; + global.warn = jest.fn(); + jest.resetModules(); + jest.mock('../src/configParameters', () => ({ + ...jest.requireActual('../src/configParameters'), + recordRuleExitStatus: jest.fn(), + })); + }); + + describe('Default config', () => { + beforeEach(async () => { + jest.doMock('../src/configParameters', () => ({ + config: { + ...originalConfig, + commitMessages: { + maxBodyLineLength: 100, + maxSummaryLength: 72, + minSummaryLength: 20, + allowedTypes: 'change,ci,docs,feat,fix,refactor,remove,revert', + }, + }, + recordRuleExitStatus, + })); + ruleCommitMessages = (await import('../src/ruleCommitMessages')).default; + }); + + it('EXPECT PASS: Message with "scope" and "body"', async () => { + danger.git.commits = [{ message: 'feat(bootloader): This is commit message with scope and body\n\nThis is a text of body' }]; + await ruleCommitMessages(); + expect(warn).not.toHaveBeenCalled(); + }); + + it('EXPECT PASS: Message with "scope", without "body"', async () => { + danger.git.commits = [{ message: 'change(wifi): This is commit message with scope without body' }]; + await ruleCommitMessages(); + expect(warn).not.toHaveBeenCalled(); + }); + + it('EXPECT PASS: Message with "scope" (with hyphen in "scope"), without "body"', async () => { + danger.git.commits = [{ message: 'change(esp-rom): This is commit message with hyphen in scope' }]; + await ruleCommitMessages(); + expect(warn).not.toHaveBeenCalled(); + }); + + it('EXPECT PASS: Message with "scope" (with asterisk in "scope"), without "body"', async () => { + danger.git.commits = [{ message: 'change(examples*storage): This is commit message with asterisk in scope' }]; + await ruleCommitMessages(); + expect(warn).not.toHaveBeenCalled(); + }); + + it('EXPECT PASS: Message with "scope" (with comma in "scope"), without "body"', async () => { + danger.git.commits = [{ message: 'change(examples,storage): This is commit message with comma in scope' }]; + await ruleCommitMessages(); + expect(warn).not.toHaveBeenCalled(); + }); + + it('EXPECT PASS: Message with "scope" (with slash in "scope"), without "body"', async () => { + danger.git.commits = [{ message: 'change(examples/storage): This is commit message with slash in scope' }]; + await ruleCommitMessages(); + expect(warn).not.toHaveBeenCalled(); + }); + + it('EXPECT PASS: Message without "scope", with "body"', async () => { + danger.git.commits = [{ message: 'change: This is commit message without scope with body\n\nThis is a text of body' }]; + await ruleCommitMessages(); + expect(warn).not.toHaveBeenCalled(); + }); + + it('EXPECT PASS: Message without "scope", without "body"', async () => { + danger.git.commits = [{ message: 'ci: This is commit message without scope and body' }]; + await ruleCommitMessages(); + expect(warn).not.toHaveBeenCalled(); + }); + + it('EXPECT PASS: Message "summary" starts with lowercase', async () => { + danger.git.commits = [{ message: 'change(rom): this message summary starts with lowercase' }]; + await ruleCommitMessages(); + expect(warn).not.toHaveBeenCalled(); + }); + + it('EXPECT PASS: Message "summary" contains text in brackets', async () => { + danger.git.commits = [{ message: 'change: this message summary starts (as usually) with lowercase' }]; + await ruleCommitMessages(); + expect(warn).not.toHaveBeenCalled(); + }); + + it('EXPECT PASS: Message with "scope", "summary" contains text in brackets', async () => { + danger.git.commits = [{ message: 'change(rom): this message summary starts (as usually) with lowercase' }]; + await ruleCommitMessages(); + expect(warn).not.toHaveBeenCalled(); + }); + + it('EXPECT PASS: Message "summary" ends with text in brackets', async () => { + danger.git.commits = [{ message: 'change: this message summary starts with lowercase (just here)' }]; + await ruleCommitMessages(); + expect(warn).not.toHaveBeenCalled(); + }); + + it('EXPECT PASS: Message with "scope", "summary" ends with text in brackets', async () => { + danger.git.commits = [{ message: 'change(rom): this message summary starts with lowercase (just here)' }]; + await ruleCommitMessages(); + expect(warn).not.toHaveBeenCalled(); + }); + + it('EXPECT FAIL: Missing colon between "type/(optional-scope)" and "summary"', async () => { + danger.git.commits = [{ message: 'change this is commit message without body' }]; + await ruleCommitMessages(); + expect(warn).toHaveBeenCalledWith(expect.stringContaining('*summary* looks empty')); + expect(warn).toHaveBeenCalledWith(expect.stringContaining('*type/action* looks empty')); + }); + + it('EXPECT FAIL: Message "summary" is too short (7 characters)', async () => { + danger.git.commits = [{ message: 'fix: Fix bug' }]; + await ruleCommitMessages(); + expect(warn).toHaveBeenCalledWith(expect.stringContaining('*summary* looks too short')); + }); + + it('EXPECT FAIL: Message "summary" is too long (88 characters)', async () => { + danger.git.commits = [{ message: 'change(rom): Refactor authentication flow for enhanced security measures and improved user experience' }]; + await ruleCommitMessages(); + expect(warn).toHaveBeenCalledWith(expect.stringContaining('*summary* appears to be too long')); + }); + + it('EXPECT FAIL: Message "summary" ends with period (full stop)', async () => { + danger.git.commits = [{ message: 'change(rom): Fixed the another bug.' }]; + await ruleCommitMessages(); + expect(warn).toHaveBeenCalledWith(expect.stringContaining('*summary* should not end with a period')); + }); + + it('EXPECT FAIL: Message "scope" starts uppercase', async () => { + danger.git.commits = [{ message: 'change(Bt): Added new feature with change\n\nThis feature adds functionality' }]; + await ruleCommitMessages(); + expect(warn).toHaveBeenCalledWith(expect.stringContaining('*scope/component* should be lowercase without whitespace')); + }); + + it('EXPECT FAIL: Message contains uppercase in "scope"', async () => { + danger.git.commits = [{ message: 'fix(dangerGH): Update token permissions - allow Danger to add comments to PR' }]; + await ruleCommitMessages(); + expect(warn).toHaveBeenCalledWith(expect.stringContaining('*scope/component* should be lowercase without whitespace')); + }); + + it('EXPECT FAIL: Message contains space in "scope"', async () => { + danger.git.commits = [{ message: 'fix(danger github): Update token permissions - allow Danger to add comments to PR' }]; + await ruleCommitMessages(); + expect(warn).toHaveBeenCalledWith(expect.stringContaining('*scope/component* should be lowercase without whitespace')); + }); + + it('EXPECT FAIL: Message with scope and body contains not allowed "type"', async () => { + danger.git.commits = [{ message: 'delete(bt): Added new feature with change\n\nThis feature adds functionality' }]; + await ruleCommitMessages(); + expect(warn).toHaveBeenCalledWith(expect.stringContaining('*type/action* should be one of')); + }); + + it('EXPECT FAIL: Message without scope and without body contains not allowed "type"', async () => { + danger.git.commits = [{ message: 'wip: Added new feature with change' }]; + await ruleCommitMessages(); + expect(warn).toHaveBeenCalledWith(expect.stringContaining('*type/action* should be one of')); + }); + + it('EXPECT FAIL: Message with scope and body contains not allowed "type" (uppercase)', async () => { + danger.git.commits = [{ message: 'Fix(bt): Added new feature with change\n\nThis feature adds functionality' }]; + await ruleCommitMessages(); + expect(warn).toHaveBeenCalledWith(expect.stringContaining('*type/action* should be one of')); + }); + + it('EXPECT FAIL: Missing blank line between "summary" and "body"', async () => { + danger.git.commits = [{ message: 'change: Added new feature with change\nThis is body without blank line' }]; + await ruleCommitMessages(); + expect(warn).toHaveBeenCalledWith(expect.stringContaining('body must have leading blank line')); + }); + + it('EXPECT FAIL: Message "body" line is too long (110 characters)', async () => { + danger.git.commits = [ + { + message: + 'fix(bt): Update database schemas\n\nUpdating the database schema to include new fields and user profile preferences, cleaning up unnecessary calls', + }, + ]; + await ruleCommitMessages(); + expect(warn).toHaveBeenCalledWith(expect.stringContaining("body's lines must not be longer than")); + }); + + it('EXPECT FAIL: Jira ticket references in commit message', async () => { + danger.git.commits = [{ message: 'fix(esp32): Fixed startup timeout issue (fixes JIRA-123)' }]; + await ruleCommitMessages(); + expect(warn).toHaveBeenCalledWith(expect.stringContaining('JIRA-123')); + expect(warn).toHaveBeenCalledWith(expect.stringContaining('remove Jira tickets')); + }); + }); + + describe('Custom config', () => { + beforeEach(async () => { + jest.doMock('../src/configParameters', () => ({ + config: { + ...originalConfig, + commitMessages: { + maxBodyLineLength: 150, + maxSummaryLength: 100, + minSummaryLength: 5, + allowedTypes: 'fix,feat,change,style,perf', + }, + }, + recordRuleExitStatus, + })); + ruleCommitMessages = (await import('../src/ruleCommitMessages')).default; + }); + + it('EXPECT PASS: Message "body" line is longer but within custom max length', async () => { + danger.git.commits = [ + { + message: + 'fix(bt): Update database schemas\n\nUpdating the database schema to include new fields and user profile preferences, cleaning up unnecessary calls and optimizing performance', + }, + ]; + await ruleCommitMessages(); + expect(warn).not.toHaveBeenCalled(); + }); + + it('EXPECT PASS: Message with "style" type', async () => { + danger.git.commits = [{ message: 'style(ui): Update button styles\n\nAdjusted padding and margins for better alignment.' }]; + await ruleCommitMessages(); + expect(warn).not.toHaveBeenCalled(); + }); + + it('EXPECT FAIL: Message "summary" is longer than custom max length', async () => { + danger.git.commits = [ + { message: 'change(rom): Refactor authentication flow for enhanced security measures and improved user experience with additional features' }, + ]; + await ruleCommitMessages(); + expect(warn).toHaveBeenCalledWith(expect.stringContaining('*summary* appears to be too long')); + }); + + it('EXPECT FAIL: Message with "fox" type', async () => { + danger.git.commits = [{ message: 'fox(ui): Fix button alignment\n\nAdjusted padding and margins for better alignment.' }]; + await ruleCommitMessages(); + expect(warn).toHaveBeenCalledWith(expect.stringContaining('*type/action* should be one of')); + }); + }); +}); diff --git a/tests/ruleNumberOfCommits.test.ts b/tests/ruleNumberOfCommits.test.ts new file mode 100644 index 0000000..d230785 --- /dev/null +++ b/tests/ruleNumberOfCommits.test.ts @@ -0,0 +1,94 @@ +import { config as originalConfig, recordRuleExitStatus } from '../src/configParameters'; + +declare global { + var danger: any; + var message: any; + var warn: any; +} + +describe('TESTS: Number of commits in Pull Request', () => { + let ruleNumberOfCommits: any; + beforeEach(() => { + global.danger = { + github: { + commits: [], + }, + }; + global.message = jest.fn(); + global.warn = jest.fn(); + jest.resetModules(); + jest.mock('../src/configParameters', () => ({ + ...jest.requireActual('../src/configParameters'), + recordRuleExitStatus: jest.fn(), + })); + }); + + describe('Default config (maxCommitsInfo: 2, maxCommitsWarning: 5)', () => { + beforeEach(async () => { + jest.doMock('../src/configParameters', () => ({ + config: { + ...originalConfig, + }, + recordRuleExitStatus, + })); + ruleNumberOfCommits = (await import('../src/ruleNumberOfCommits')).default; + }); + it('EXPECT PASS: Not too many (2) commits', () => { + global.danger.github.commits.length = 2; + ruleNumberOfCommits(); + expect(message).not.toHaveBeenCalled(); + expect(warn).not.toHaveBeenCalled(); + }); + + it('EXPECT FAIL with MESSAGE: Too many (3) commits (exceeded soft limit)', () => { + global.danger.github.commits.length = 3; + ruleNumberOfCommits(); + expect(message).toHaveBeenCalledWith(expect.stringContaining('You might consider squashing')); + expect(warn).not.toHaveBeenCalled(); + }); + + it('EXPECT FAIL with WARN: Too many (6) commits (exceeded hard limit)', () => { + global.danger.github.commits.length = 6; + ruleNumberOfCommits(); + expect(message).not.toHaveBeenCalled(); + expect(warn).toHaveBeenCalledWith(expect.stringContaining('Please consider squashing')); + }); + }); + + describe('Custom config (maxCommitsInfo: 5, maxCommitsWarning: 7)', () => { + beforeEach(async () => { + jest.doMock('../src/configParameters', () => ({ + config: { + ...originalConfig, + numberOfCommits: { + maxCommitsInfo: 5, + maxCommitsWarning: 7, + }, + }, + recordRuleExitStatus, + })); + ruleNumberOfCommits = (await import('../src/ruleNumberOfCommits')).default; + }); + + it('EXPECT PASS: Not too many (4) commits', () => { + global.danger.github.commits.length = 4; + ruleNumberOfCommits(); + expect(message).not.toHaveBeenCalled(); + expect(warn).not.toHaveBeenCalled(); + }); + + it('EXPECT FAIL with MESSAGE: Too many (6) commits (exceeded custom soft limit)', () => { + global.danger.github.commits.length = 6; + ruleNumberOfCommits(); + expect(message).toHaveBeenCalledWith(expect.stringContaining('You might consider squashing')); + expect(warn).not.toHaveBeenCalled(); + }); + + it('EXPECT FAIL with WARN: Too many (8) commits (exceeded custom hard limit)', () => { + global.danger.github.commits.length = 8; + ruleNumberOfCommits(); + expect(message).not.toHaveBeenCalled(); + expect(warn).toHaveBeenCalledWith(expect.stringContaining('Please consider squashing')); + }); + }); +}); diff --git a/tests/rulePrDescription.test.ts b/tests/rulePrDescription.test.ts new file mode 100644 index 0000000..08bfbc2 --- /dev/null +++ b/tests/rulePrDescription.test.ts @@ -0,0 +1,324 @@ +import { config as originalConfig, recordRuleExitStatus } from '../src/configParameters'; + +declare global { + var danger: any; + var warn: any; +} + +describe('TESTS: Pull Request sufficient Description', () => { + let rulePrDescription: any; + beforeEach(() => { + global.danger = { + github: { + pr: { + body: '', + }, + }, + }; + global.warn = jest.fn(); + jest.resetModules(); + jest.mock('../src/configParameters', () => ({ + ...jest.requireActual('../src/configParameters'), + recordRuleExitStatus: jest.fn(), + })); + }); + + describe('DEFAULT CONFIG: Required length 50, ignored sections: "Release notes", "Related", "Breaking change notes"', () => { + beforeEach(async () => { + jest.doMock('../src/configParameters', () => ({ + config: { + ...originalConfig, + prDescription: { + minLength: 50, + ignoredSections: 'related,release,breaking', + }, + }, + recordRuleExitStatus, + })); + rulePrDescription = (await import('../src/rulePrDescription')).default; + }); + + it('EXPECT PASS: Description without headers (80 characters)', () => { + danger.github.pr.body = `This is a long description that is certainly over the threshold of 50 characters`; + rulePrDescription(); + expect(warn).not.toHaveBeenCalled(); + }); + + it('EXPECT PASS: Description with header "# Description" on the beginning', () => { + danger.github.pr.body = `# Description + This is a long description that is certainly over the threshold of 50 characters`; + rulePrDescription(); + expect(warn).not.toHaveBeenCalled(); + }); + + it('EXPECT PASS: Description with section header "# Description" on the beginning and with ignored section "Related"', () => { + danger.github.pr.body = `# Description + This PR solves the problem of the BOT being an 'admin' in the newly created GitHub project. + The 'admin' role is not actually required for 'espressif-bot', but is automatically created this way because the BOT token + is the one used to create the GitHub project (bot becomes admin because bot created the project). + As a last step of creating a new GitHub project, a method was added to downgrade the BOT role from 'Admin' to 'Write'. + + ## Related + - RDT-490 + - https://github.com/espressif/test-project-bot/pull/15`; + rulePrDescription(); + expect(warn).not.toHaveBeenCalled(); + }); + + it('EXPECT PASS: Description without section header on the beginning but with ignored section "Related"', () => { + danger.github.pr.body = `This PR solves the problem of the BOT being an 'admin' in the newly created GitHub project. + The 'admin' role is not actually required for 'espressif-bot', but is automatically created this way because the BOT token is the one used to create the GitHub project (bot becomes admin because bot created the project). + As a last step of creating a new GitHub project, a method was added to downgrade the BOT role from 'Admin' to 'Write'. + + ## Related + - RDT-490 + - https://github.com/espressif/test-project-bot/pull/15`; + rulePrDescription(); + expect(warn).not.toHaveBeenCalled(); + }); + + it('EXPECT PASS: Starts with text, then multiple ignored sections ("Related", "Breaking changes)', () => { + danger.github.pr.body = `This PR solves the problem of the BOT being an 'admin' in the newly created GitHub project. The 'admin' role is not actually required for 'espressif-bot', but is automatically created this way because the BOT token is the one used to create the GitHub project (bot becomes admin because bot created the project). + As a last step of creating a new GitHub project, a method was added to downgrade the BOT role from 'Admin' to 'Write'. + + ## Breaking changes + - this is maybe a breaking change + + ## Related + - RDT-490 + - https://github.com/espressif/test-project-bot/pull/15`; + rulePrDescription(); + expect(warn).not.toHaveBeenCalled(); + }); + + it('EXPECT FAIL: Description is null', () => { + danger.github.pr.body = null; + rulePrDescription(); + expect(warn).toHaveBeenCalledWith(expect.stringContaining('The Pull Request description is empty')); + }); + + it('EXPECT FAIL: Empty description, only newlines', () => { + danger.github.pr.body = '\n\n\n\n\n\n\n\n'; + rulePrDescription(); + expect(warn).toHaveBeenCalledWith(expect.stringContaining('The Pull Request description is empty')); + }); + + it('EXPECT FAIL: Short description (9 characters)', () => { + danger.github.pr.body = 'Fixed bug'; + rulePrDescription(); + expect(warn).toHaveBeenCalledWith(expect.stringContaining('The Pull Request description looks very brief')); + }); + + it('EXPECT FAIL: Short description (only ignored section "Related")', () => { + danger.github.pr.body = `## Related + - RDT-490 + - https://github.com/espressif/test-project-bot/pull/15`; + rulePrDescription(); + expect(warn).toHaveBeenCalledWith(expect.stringContaining('The Pull Request description looks very brief')); + }); + + it('EXPECT FAIL: Short description (22 characters), ignored section "Related" and HTML comments', () => { + danger.github.pr.body = `Fixed bug + + ## Related + - RDT-490 + - https://github.com/espressif/test-project-bot/pull/15`; + rulePrDescription(); + expect(warn).toHaveBeenCalledWith(expect.stringContaining('The Pull Request description looks very brief')); + }); + + it('EXPECT FAIL: Only HTML comments and ignored sections (minimum ESP-IDF Gitlab MR template)', () => { + danger.github.pr.body = ` + + + + + + + ## Related + + + ## Release notes + + + `; + rulePrDescription(); + expect(warn).toHaveBeenCalledWith(expect.stringContaining('The Pull Request description looks very brief')); + }); + + it('EXPECT FAIL: Short PR Description with ignored sections and HTML comments', async () => { + danger.github.pr.body = ` + ## Description + PR description + + ## Related + + - JIRA-123 + - Closes JIRA-888 + + ## Release notes + - This is PR description test text in section Release notes + - [Area] This is PR description test text in section Release notes + + ## Breaking change notes + - This is PR description test text in section Breaking change notes`; + rulePrDescription(); + expect(warn).toHaveBeenCalledWith(expect.stringContaining('The Pull Request description looks very brief')); + }); + }); + + describe('CUSTOM CONFIG: Required length 2000, ignored sections: "Release notes", "Related", "Breaking change notes"', () => { + beforeEach(async () => { + jest.doMock('../src/configParameters', () => ({ + config: { + ...originalConfig, + prDescription: { + minLength: 2000, + ignoredSections: 'related,release,breaking', + }, + }, + recordRuleExitStatus, + })); + rulePrDescription = (await import('../src/rulePrDescription')).default; + }); + + it('EXPECT PASS: Description length is more than 2000 characters', async () => { + danger.github.pr.body = 'A'.repeat(2100); // A string with 2100 'A' characters + rulePrDescription(); + expect(global.warn).not.toHaveBeenCalled(); + }); + + it('EXPECT FAIL: Description length is less than 2000 characters', async () => { + danger.github.pr.body = 'This is a description that is certainly under the threshold of 2000 characters'; + rulePrDescription(); + expect(warn).toHaveBeenCalledWith(expect.stringContaining('The Pull Request description looks very brief')); + }); + }); + + describe('CUSTOM CONFIG: Required length 100, no ignored sections', () => { + beforeEach(async () => { + jest.doMock('../src/configParameters', () => ({ + config: { + ...originalConfig, + prDescription: { + minLength: 100, + ignoredSections: '', + }, + }, + recordRuleExitStatus, + })); + rulePrDescription = (await import('../src/rulePrDescription')).default; + }); + + it('EXPECT PASS: Description with no ignored section, has 323 chars total', async () => { + danger.github.pr.body = ` + ## Description + This is PR description test text in section description + + ## Related + + - JIRA-123 + - Closes JIRA-888 + + ## Release notes + - This is PR description test text in section Release notes + - [Area] This is PR description test text in section Release notes + + ## Breaking change notes + - This is PR description test text in section Breaking change notes + `; + rulePrDescription(); + expect(global.warn).not.toHaveBeenCalled(); + }); + + it('EXPECT FAIL: Only HTML comments and kept sections headers (minimum ESP-IDF PR template)', async () => { + danger.github.pr.body = ` + + + + + + + ## Related + + + ## Release notes + + + + `; + rulePrDescription(); + expect(warn).toHaveBeenCalledWith(expect.stringContaining('The Pull Request description looks very brief')); + }); + }); + describe('CUSTOM CONFIG: Required length 400, ignored sections: "Release"', () => { + beforeEach(async () => { + jest.doMock('../src/configParameters', () => ({ + config: { + ...originalConfig, + prDescription: { + minLength: 400, + ignoredSections: 'release', + }, + }, + recordRuleExitStatus, + })); + rulePrDescription = (await import('../src/rulePrDescription')).default; + }); + + it('EXPECT PASS: Description with ignoring "Related ..." section, has 430 chars after', async () => { + danger.github.pr.body = `This is PR description test text + ## Description + This is PR description test text in section description + + ## Related + + - JIRA-123 + - Closes JIRA-888 + + ## Release notes + - This is PR description test text in section Release notes + - [Area] This is PR description test text in section Release notes + + ## Testing + + - This is PR description test text in section Testing\ g + + ## Breaking change notes + - This is PR description test text in section Breaking change notes + + ## Other section + - This is PR description test text in section Other section + `; + rulePrDescription(); + expect(global.warn).not.toHaveBeenCalled(); + }); + + it('EXPECT FAIL: Description with ignoring "Related ..." section, has 346 chars after', async () => { + danger.github.pr.body = `This is PR description test text + ## Description + This is PR description test text in section description + + ## Related + + - JIRA-123 + - Closes JIRA-888 + + ## Release notes + - This is PR description test text in section Release notes + - [Area] This is PR description test text in section Release notes + + ## Testing + + - This is PR description test text in section Testing\ g + + ## Breaking change notes + - This is PR description test text in section Breaking change notes + `; + rulePrDescription(); + expect(warn).toHaveBeenCalledWith(expect.stringContaining('The Pull Request description looks very brief')); + }); + }); +}); diff --git a/tests/rulePrSize.test.ts b/tests/rulePrSize.test.ts new file mode 100644 index 0000000..640fa50 --- /dev/null +++ b/tests/rulePrSize.test.ts @@ -0,0 +1,84 @@ +import { config as originalConfig, recordRuleExitStatus } from '../src/configParameters'; + +declare global { + var danger: any; + var message: any; +} + +describe('TESTS: Merge request size (number of changed lines)', () => { + let rulePrSize: any; + beforeEach(() => { + global.danger = { + github: { + pr: { + additions: 0, + deletions: 0, + }, + }, + }; + global.message = jest.fn(); + jest.resetModules(); + jest.mock('../src/configParameters', () => ({ + ...jest.requireActual('../src/configParameters'), + recordRuleExitStatus: jest.fn(), + })); + }); + + describe('Default config (maxChangedLines: 1000)', () => { + beforeEach(async () => { + jest.doMock('../src/configParameters', () => ({ + config: { + ...originalConfig, + prSize: { + maxChangedLines: 1000, + }, + }, + recordRuleExitStatus, + })); + rulePrSize = (await import('../src/rulePrSize')).default; + }); + + it('EXPECT PASS: MR has not too many (800) lines of code', async () => { + danger.github.pr.additions = 800; + danger.github.pr.deletions = 0; + await rulePrSize(); + expect(message).not.toHaveBeenCalled(); + }); + + it('EXPECT FAIL: MR has too many (1200) lines of code', async () => { + danger.github.pr.additions = 1200; + danger.github.pr.deletions = 0; + await rulePrSize(); + expect(message).toHaveBeenCalledWith(expect.stringContaining('This PR seems to be quite large')); + }); + }); + + describe('Custom config (maxChangedLines: 2000)', () => { + beforeEach(async () => { + jest.doMock('../src/configParameters', () => ({ + config: { + ...originalConfig, + prSize: { + maxChangedLines: 2000, + }, + }, + recordRuleExitStatus, + })); + rulePrSize = (await import('../src/rulePrSize')).default; + }); + + it('EXPECT PASS: MR has not too many (1500) lines of code', async () => { + danger.github.pr.additions = 1500; + danger.github.pr.deletions = 0; + await rulePrSize(); + expect(message).not.toHaveBeenCalled(); + }); + + it('EXPECT FAIL: MR has too many (2500) lines of code', async () => { + danger.github.pr.additions = 1500; + danger.github.pr.deletions = 1000; + await rulePrSize(); + expect(message).toHaveBeenCalledWith(expect.stringContaining('This PR seems to be quite large')); + }); + }); +}); diff --git a/tests/ruleSourceBranchName.test.ts b/tests/ruleSourceBranchName.test.ts new file mode 100644 index 0000000..3ddc484 --- /dev/null +++ b/tests/ruleSourceBranchName.test.ts @@ -0,0 +1,51 @@ +declare global { + var danger: any; + var warn: any; +} + +describe('TESTS: Source branch name', () => { + let ruleSourceBranchName: any; + beforeEach(async () => { + global.danger = { + github: { pr: { head: { ref: '' } } }, + }; + global.warn = jest.fn(); + jest.resetModules(); + jest.mock('../src/configParameters', () => ({ + ...jest.requireActual('../src/configParameters'), + recordRuleExitStatus: jest.fn(), + })); + ruleSourceBranchName = (await import('../src/ruleSourceBranchName')).default; + }); + + it('EXPECT PASS: Source branch name contains no slashes', () => { + danger.github.pr.head.ref = 'feature-fix_something'; + ruleSourceBranchName(); + expect(warn).not.toHaveBeenCalled(); + }); + + it('EXPECT PASS: Source branch name is valid (one slash, no uppercase)', () => { + danger.github.pr.head.ref = 'feature/fix_something'; + ruleSourceBranchName(); + expect(warn).not.toHaveBeenCalled(); + }); + + it('EXPECT FAIL: Source branch name contains more than one slash', () => { + danger.github.pr.head.ref = 'feature/some/nested/thing/fix'; + ruleSourceBranchName(); + expect(warn).toHaveBeenCalledWith(expect.stringContaining('contains more than one slash')); + }); + + it('EXPECT FAIL: Source branch name contains uppercase letters', () => { + danger.github.pr.head.ref = 'docs/add_CN_chapter_for_BLE_API'; + ruleSourceBranchName(); + expect(warn).toHaveBeenCalledWith(expect.stringContaining('contains uppercase letters')); + }); + + it('EXPECT FAIL: Source branch name contains more than one slash and uppercase letters', () => { + danger.github.pr.head.ref = 'Feature/New/Feature'; + ruleSourceBranchName(); + expect(warn).toHaveBeenCalledWith(expect.stringContaining('contains more than one slash')); + }); +}); +export {}; diff --git a/tests/ruleTargetBranch.test.ts b/tests/ruleTargetBranch.test.ts new file mode 100644 index 0000000..f24d6f3 --- /dev/null +++ b/tests/ruleTargetBranch.test.ts @@ -0,0 +1,49 @@ +declare global { + var danger: any; + var warn: any; +} + +describe('TESTS: Target Branch Check', () => { + let ruleTargetBranch: any; + beforeEach(async () => { + global.danger = { + github: { + pr: { + base: { + ref: '', + repo: { + owner: { login: 'ownerLogin' }, + name: 'repoName', + }, + }, + }, + }, + }; + global.warn = jest.fn(); + jest.resetModules(); + jest.mock('@octokit/rest', () => ({ + Octokit: jest.fn().mockImplementation(() => ({ + repos: { + get: jest.fn().mockResolvedValue({ + data: { default_branch: 'main' }, + }), + }, + })), + })); + ruleTargetBranch = (await import('../src/ruleTargetBranch')).default; // Adjust the import path according to your project structure + }); + + it('EXPECT PASS: Target branch is the default branch', async () => { + danger.github.pr.base.ref = 'main'; + await ruleTargetBranch(); + expect(warn).not.toHaveBeenCalled(); + }); + + it('EXPECT FAIL: Target branch is not the default branch', async () => { + danger.github.pr.base.ref = 'develop'; + await ruleTargetBranch(); + expect(warn).toHaveBeenCalledWith(expect.stringContaining('The **target branch** for this Pull Request **must be the default branch** of the project')); + }); +}); + +export {};