Skip to content
Merged
Show file tree
Hide file tree
Changes from 8 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions .github/workflows/danger.yml
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,10 @@ jobs:
with:
fetch-depth: 0

- name: Download dangerfile.js
run: wget https://raw.githubusercontent.com/getsentry/github-workflows/${{ inputs._workflow_version }}/danger/dangerfile.js -P ${{ runner.temp }}
- name: Download dangerfile.js and utilities
run: |
wget https://raw.githubusercontent.com/getsentry/github-workflows/${{ inputs._workflow_version }}/danger/dangerfile.js -P ${{ runner.temp }}
wget https://raw.githubusercontent.com/getsentry/github-workflows/${{ inputs._workflow_version }}/danger/dangerfile-utils.js -P ${{ runner.temp }}
# Using a pre-built docker image in GitHub container registry instaed of NPM to reduce possible attack vectors.
- name: Run DangerJS
Expand Down
20 changes: 20 additions & 0 deletions .github/workflows/script-tests.yml
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
# This isn't a reusable workflow but a CI action for this repo itself - testing the contained workflows & scripts.
name: Script Tests
permissions:
contents: read

on:
push:
Expand All @@ -23,3 +25,21 @@ jobs:
- run: Invoke-Pester
working-directory: updater
shell: pwsh

danger:
name: Danger JS Tests
runs-on: ubuntu-latest
defaults:
run:
working-directory: danger
steps:
- uses: actions/checkout@v4

- uses: actions/setup-node@v4
with:
node-version: '18'

- run: node --test

- name: Check syntax
run: node -c dangerfile.js
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

### Features

- Danger - Improve testing infrastructure and conventional commit scope handling ([#105](https://github.com/getsentry/github-workflows/pull/105))
- Add Proguard artifact endpoint for Android builds in sentry-server ([#100](https://github.com/getsentry/github-workflows/pull/100))

### Security
Expand Down
79 changes: 79 additions & 0 deletions danger/dangerfile-utils.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,79 @@
/// Unified configuration for PR flavors (based on real Sentry usage analysis)
const FLAVOR_CONFIG = [
{
labels: ["feat", "feature"],
changelog: "Features",
isFeature: true
},
{
labels: ["fix", "bug", "bugfix"],
changelog: "Fixes"
},
{
labels: ["sec", "security"],
changelog: "Security"
},
{
labels: ["perf", "performance"],
changelog: "Performance"
},
{
// Internal changes - no changelog needed
changelog: undefined,
labels: [
"docs",
"doc",
"style",
"ref",
"refactor",
"tests",
"test",
"build",
"ci",
"chore",
"meta",
"deps",
"dep"
]
}
];

/// Get flavor configuration for a given PR flavor
function getFlavorConfig(prFlavor) {
const normalizedFlavor = prFlavor.toLowerCase().trim();

// Strip scope/context from conventional commit format: "type(scope)" -> "type"
const baseType = normalizedFlavor.replace(/\([^)]*\)/, '');

const config = FLAVOR_CONFIG.find(config =>
config.labels.includes(normalizedFlavor) || config.labels.includes(baseType)
);

return config || {
changelog: "Features" // Default to Features
};
}


/// Extract PR flavor from title or branch name
function extractPRFlavor(prTitle, prBranchRef) {
if (prTitle) {
const parts = prTitle.split(":");
if (parts.length > 1) {
return parts[0].toLowerCase().trim();
}
}
if (prBranchRef) {
const parts = prBranchRef.split("/");
if (parts.length > 1) {
return parts[0].toLowerCase();
}
}
return "";
}

module.exports = {
FLAVOR_CONFIG,
getFlavorConfig,
extractPRFlavor
};
184 changes: 184 additions & 0 deletions danger/dangerfile-utils.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,184 @@
const { describe, it } = require('node:test');
const assert = require('node:assert');
const { getFlavorConfig, extractPRFlavor, FLAVOR_CONFIG } = require('./dangerfile-utils.js');

describe('dangerfile-utils', () => {
describe('getFlavorConfig', () => {
it('should return config for features with isFeature true', () => {
const featConfig = getFlavorConfig('feat');
assert.strictEqual(featConfig.changelog, 'Features');
assert.strictEqual(featConfig.isFeature, true);

const featureConfig = getFlavorConfig('feature');
assert.strictEqual(featureConfig.changelog, 'Features');
assert.strictEqual(featureConfig.isFeature, true);
});

it('should return config for fixes without isFeature', () => {
const fixConfig = getFlavorConfig('fix');
assert.strictEqual(fixConfig.changelog, 'Fixes');
assert.strictEqual(fixConfig.isFeature, undefined);

const bugConfig = getFlavorConfig('bug');
assert.strictEqual(bugConfig.changelog, 'Fixes');
assert.strictEqual(bugConfig.isFeature, undefined);

const bugfixConfig = getFlavorConfig('bugfix');
assert.strictEqual(bugfixConfig.changelog, 'Fixes');
assert.strictEqual(bugfixConfig.isFeature, undefined);
});

it('should return config with undefined changelog for skipped flavors', () => {
const skipFlavors = ['docs', 'doc', 'ci', 'tests', 'test', 'style', 'refactor', 'build', 'chore', 'meta', 'deps', 'dep', 'chore(deps)', 'build(deps)'];

skipFlavors.forEach(flavor => {
const config = getFlavorConfig(flavor);
assert.strictEqual(config.changelog, undefined, `${flavor} should have undefined changelog`);
assert.strictEqual(config.isFeature, undefined, `${flavor} should have undefined isFeature`);
});
});

it('should return default config for unknown flavors', () => {
const unknownConfig = getFlavorConfig('unknown');
assert.strictEqual(unknownConfig.changelog, 'Features');
assert.strictEqual(unknownConfig.isFeature, undefined);

const emptyConfig = getFlavorConfig('');
assert.strictEqual(emptyConfig.changelog, 'Features');
assert.strictEqual(emptyConfig.isFeature, undefined);
});

it('should be case-insensitive and handle whitespace', () => {
const config1 = getFlavorConfig('FEAT');
assert.strictEqual(config1.changelog, 'Features');

const config2 = getFlavorConfig(' fix ');
assert.strictEqual(config2.changelog, 'Fixes');
});

it('should handle all security-related flavors', () => {
const secConfig = getFlavorConfig('sec');
assert.strictEqual(secConfig.changelog, 'Security');

const securityConfig = getFlavorConfig('security');
assert.strictEqual(securityConfig.changelog, 'Security');
});

it('should handle all performance-related flavors', () => {
const perfConfig = getFlavorConfig('perf');
assert.strictEqual(perfConfig.changelog, 'Performance');

const performanceConfig = getFlavorConfig('performance');
assert.strictEqual(performanceConfig.changelog, 'Performance');
});

it('should handle ref flavor (internal changes - no changelog)', () => {
const refConfig = getFlavorConfig('ref');
assert.strictEqual(refConfig.changelog, undefined);
assert.strictEqual(refConfig.isFeature, undefined);
});

it('should handle scoped flavors by stripping scope', () => {
const scopedFeat = getFlavorConfig('feat(core)');
assert.strictEqual(scopedFeat.changelog, 'Features');
assert.strictEqual(scopedFeat.isFeature, true);

const scopedFix = getFlavorConfig('fix(browser)');
assert.strictEqual(scopedFix.changelog, 'Fixes');
assert.strictEqual(scopedFix.isFeature, undefined);

const scopedChore = getFlavorConfig('chore(deps)');
assert.strictEqual(scopedChore.changelog, undefined);
});
});

describe('extractPRFlavor', () => {
it('should extract flavor from PR title with colon', () => {
const flavor = extractPRFlavor('feat: add new feature', null);
assert.strictEqual(flavor, 'feat');

const flavor2 = extractPRFlavor('Fix: resolve bug in authentication', null);
assert.strictEqual(flavor2, 'fix');

const flavor3 = extractPRFlavor('Docs: Update readme', null);
assert.strictEqual(flavor3, 'docs');
});

it('should extract flavor from branch name with slash', () => {
const flavor = extractPRFlavor(null, 'feature/new-api');
assert.strictEqual(flavor, 'feature');

const flavor2 = extractPRFlavor(null, 'ci/update-workflows');
assert.strictEqual(flavor2, 'ci');

const flavor3 = extractPRFlavor(null, 'fix/auth-bug');
assert.strictEqual(flavor3, 'fix');
});

it('should prefer title over branch if both available', () => {
const flavor = extractPRFlavor('feat: add feature', 'ci/update-workflows');
assert.strictEqual(flavor, 'feat');
});

it('should return empty string if no flavor found', () => {
const flavor1 = extractPRFlavor('Simple title', null);
assert.strictEqual(flavor1, '');

const flavor2 = extractPRFlavor(null, 'simple-branch');
assert.strictEqual(flavor2, '');

const flavor3 = extractPRFlavor(null, null);
assert.strictEqual(flavor3, '');
});

it('should handle edge cases', () => {
const flavor1 = extractPRFlavor(':', null);
assert.strictEqual(flavor1, '');

const flavor2 = extractPRFlavor(null, '/');
assert.strictEqual(flavor2, '');

const flavor3 = extractPRFlavor('title: with: multiple: colons', null);
assert.strictEqual(flavor3, 'title');
});
});


describe('FLAVOR_CONFIG integrity', () => {
it('should have unique labels across all configs', () => {
const allLabels = [];
FLAVOR_CONFIG.forEach(config => {
config.labels.forEach(label => {
assert.ok(!allLabels.includes(label), `Duplicate label found: ${label}`);
allLabels.push(label);
});
});
});

it('should have proper structure for all configs', () => {
FLAVOR_CONFIG.forEach((config, index) => {
assert.ok(Array.isArray(config.labels), `Config ${index} should have labels array`);
assert.ok(config.labels.length > 0, `Config ${index} should have at least one label`);
assert.ok(config.hasOwnProperty('changelog'), `Config ${index} should have changelog property`);

// changelog should be either a string or undefined
if (config.changelog !== undefined) {
assert.strictEqual(typeof config.changelog, 'string', `Config ${index} changelog should be string or undefined`);
}

// isFeature should be true or undefined (not false)
if (config.hasOwnProperty('isFeature')) {
assert.strictEqual(config.isFeature, true, `Config ${index} isFeature should be true or undefined`);
}
});
});

it('should have only Features configs with isFeature true', () => {
FLAVOR_CONFIG.forEach(config => {
if (config.isFeature === true) {
assert.strictEqual(config.changelog, 'Features', 'Only Features configs should have isFeature true');
}
});
});
});
});
38 changes: 18 additions & 20 deletions danger/dangerfile.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
const { getFlavorConfig, extractPRFlavor } = require('./dangerfile-utils.js');

const headRepoName = danger.github.pr.head.repo.git_url;
const baseRepoName = danger.github.pr.base.repo.git_url;
const isFork = headRepoName != baseRepoName;
Expand Down Expand Up @@ -36,27 +38,15 @@ if (isFork) {

// e.g. "feat" if PR title is "Feat : add more useful stuff"
// or "ci" if PR branch is "ci/update-danger"
const prFlavor = (function () {
if (danger.github && danger.github.pr) {
if (danger.github.pr.title) {
const parts = danger.github.pr.title.split(":");
if (parts.length > 1) {
return parts[0].toLowerCase().trim();
}
}
if (danger.github.pr.head && danger.github.pr.head.ref) {
const parts = danger.github.pr.head.ref.split("/");
if (parts.length > 1) {
return parts[0].toLowerCase();
}
}
}
return "";
})();
const prFlavor = extractPRFlavor(
danger.github?.pr?.title,
danger.github?.pr?.head?.ref
);
console.log(`::debug:: PR Flavor: '${prFlavor}'`);

async function checkDocs() {
if (prFlavor.startsWith("feat")) {
const flavorConfig = getFlavorConfig(prFlavor);
if (flavorConfig.isFeature) {
message(
'Do not forget to update <a href="https://github.com/getsentry/sentry-docs">Sentry-docs</a> with your feature once the pull request gets approved.'
);
Expand All @@ -65,10 +55,11 @@ async function checkDocs() {

async function checkChangelog() {
const changelogFile = "CHANGELOG.md";
const flavorConfig = getFlavorConfig(prFlavor);

// Check if skipped
// Check if skipped - either by flavor config, explicit skip, or skip label
if (
["ci", "test", "deps", "chore(deps)", "build(deps)"].includes(prFlavor) ||
flavorConfig.changelog === undefined ||
(danger.github.pr.body + "").includes("#skip-changelog") ||
(danger.github.pr.labels || []).some(label => label.name === 'skip-changelog')
) {
Expand Down Expand Up @@ -103,6 +94,7 @@ async function checkChangelog() {
}
}


/// Report missing changelog entry
function reportMissingChangelog(changelogFile) {
fail("Please consider adding a changelog entry for the next release.", changelogFile);
Expand All @@ -113,6 +105,10 @@ function reportMissingChangelog(changelogFile) {
.trim()
.replace(/\.+$/, "");

// Determine the appropriate section based on PR flavor
const flavorConfig = getFlavorConfig(prFlavor);
const sectionName = flavorConfig.changelog || "Features";

markdown(
`
### Instructions and example for changelog
Expand All @@ -124,6 +120,8 @@ Example:
\`\`\`markdown
## Unreleased

### ${sectionName}

- ${prTitleFormatted} ([#${danger.github.pr.number}](${danger.github.pr.html_url}))
\`\`\`

Expand Down
Loading