mirror of
https://github.com/esphome/esphome.git
synced 2026-08-22 22:26:21 +00:00
[ci] Tell stacked pull requests apart from hand-built chains (#17980)
This commit is contained in:
@@ -13,6 +13,7 @@ module.exports = {
|
|||||||
'merging-to-release',
|
'merging-to-release',
|
||||||
'merging-to-beta',
|
'merging-to-beta',
|
||||||
'chained-pr',
|
'chained-pr',
|
||||||
|
'stacked-pr',
|
||||||
'core',
|
'core',
|
||||||
'small-pr',
|
'small-pr',
|
||||||
'medium-pr',
|
'medium-pr',
|
||||||
|
|||||||
@@ -33,8 +33,41 @@ async function fetchPrFileContent(github, context, path) {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// Check whether a pull request is part of a GitHub stack.
|
||||||
|
//
|
||||||
|
// GitHub's stacked pull request feature adds a `stack` object to the pull
|
||||||
|
// request resource. It is present on every pull request in the stack -
|
||||||
|
// including the bottom one, whose base is already `dev` - and is absent
|
||||||
|
// entirely on standalone pull requests.
|
||||||
|
//
|
||||||
|
// The `pull_request_target` webhook payload is not guaranteed to carry this
|
||||||
|
// field, so fall back to asking the API when it is missing. Guessing wrong
|
||||||
|
// here is costly: a stacked pull request mistaken for a manually chained one
|
||||||
|
// gets a label that blocks merging.
|
||||||
|
async function isStackedPr(github, context) {
|
||||||
|
const pr = context.payload.pull_request;
|
||||||
|
if (pr.stack != null) {
|
||||||
|
return true;
|
||||||
|
}
|
||||||
|
|
||||||
|
try {
|
||||||
|
const { owner, repo } = context.repo;
|
||||||
|
const { data } = await github.rest.pulls.get({
|
||||||
|
owner,
|
||||||
|
repo,
|
||||||
|
pull_number: pr.number,
|
||||||
|
});
|
||||||
|
return data.stack != null;
|
||||||
|
} catch (error) {
|
||||||
|
// Treat an API failure as "not stacked" so a chained pull request still
|
||||||
|
// gets its blocking label rather than silently slipping through.
|
||||||
|
console.log('Failed to check stack membership:', error.message);
|
||||||
|
return false;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
// Strategy: Merge branch detection
|
// Strategy: Merge branch detection
|
||||||
async function detectMergeBranch(context) {
|
async function detectMergeBranch(github, context) {
|
||||||
const labels = new Set();
|
const labels = new Set();
|
||||||
const baseRef = context.payload.pull_request.base.ref;
|
const baseRef = context.payload.pull_request.base.ref;
|
||||||
|
|
||||||
@@ -42,7 +75,11 @@ async function detectMergeBranch(context) {
|
|||||||
labels.add('merging-to-release');
|
labels.add('merging-to-release');
|
||||||
} else if (baseRef === 'beta') {
|
} else if (baseRef === 'beta') {
|
||||||
labels.add('merging-to-beta');
|
labels.add('merging-to-beta');
|
||||||
|
} else if (await isStackedPr(github, context)) {
|
||||||
|
// GitHub manages the merge order for a stack, so these are not blocked.
|
||||||
|
labels.add('stacked-pr');
|
||||||
} else if (baseRef !== 'dev') {
|
} else if (baseRef !== 'dev') {
|
||||||
|
// A chain built by hand: it must not merge until its base branch does.
|
||||||
labels.add('chained-pr');
|
labels.add('chained-pr');
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -88,7 +88,7 @@ module.exports = async ({ github, context }) => {
|
|||||||
|
|
||||||
// Early exit for release and beta branches only
|
// Early exit for release and beta branches only
|
||||||
if (baseRef === 'release' || baseRef === 'beta') {
|
if (baseRef === 'release' || baseRef === 'beta') {
|
||||||
const branchLabels = await detectMergeBranch(context);
|
const branchLabels = await detectMergeBranch(github, context);
|
||||||
const finalLabels = Array.from(branchLabels);
|
const finalLabels = Array.from(branchLabels);
|
||||||
|
|
||||||
console.log('Computed labels (merge branch only):', finalLabels.join(', '));
|
console.log('Computed labels (merge branch only):', finalLabels.join(', '));
|
||||||
@@ -118,7 +118,7 @@ module.exports = async ({ github, context }) => {
|
|||||||
deprecatedResult,
|
deprecatedResult,
|
||||||
maintainerAccess
|
maintainerAccess
|
||||||
] = await Promise.all([
|
] = await Promise.all([
|
||||||
detectMergeBranch(context),
|
detectMergeBranch(github, context),
|
||||||
detectComponentPlatforms(changedFiles, apiData),
|
detectComponentPlatforms(changedFiles, apiData),
|
||||||
detectNewComponents(github, context, prFiles),
|
detectNewComponents(github, context, prFiles),
|
||||||
detectNewPlatforms(github, context, prFiles, apiData),
|
detectNewPlatforms(github, context, prFiles, apiData),
|
||||||
|
|||||||
@@ -1,6 +1,7 @@
|
|||||||
const { describe, it } = require('node:test');
|
const { describe, it } = require('node:test');
|
||||||
const assert = require('node:assert/strict');
|
const assert = require('node:assert/strict');
|
||||||
const {
|
const {
|
||||||
|
detectMergeBranch,
|
||||||
detectNewPlatforms,
|
detectNewPlatforms,
|
||||||
detectNewComponents,
|
detectNewComponents,
|
||||||
detectPRSize,
|
detectPRSize,
|
||||||
@@ -36,6 +37,107 @@ const API_DATA = {
|
|||||||
const WITH_SCHEMA = 'CONFIG_SCHEMA = cv.Schema({})';
|
const WITH_SCHEMA = 'CONFIG_SCHEMA = cv.Schema({})';
|
||||||
const WITHOUT_SCHEMA = 'CODEOWNERS = ["@esphome/core"]';
|
const WITHOUT_SCHEMA = 'CODEOWNERS = ["@esphome/core"]';
|
||||||
|
|
||||||
|
// ---------------------------------------------------------------------------
|
||||||
|
// detectMergeBranch
|
||||||
|
// ---------------------------------------------------------------------------
|
||||||
|
|
||||||
|
// Builds a fresh context for detectMergeBranch tests instead of mutating the
|
||||||
|
// shared CONTEXT fixture above (which other describe blocks rely on).
|
||||||
|
function makeMergeContext(baseRef, { stack } = {}) {
|
||||||
|
const pull_request = { number: 1, base: { ref: baseRef } };
|
||||||
|
if (stack !== undefined) {
|
||||||
|
pull_request.stack = stack;
|
||||||
|
}
|
||||||
|
return {
|
||||||
|
repo: { owner: 'esphome', repo: 'esphome' },
|
||||||
|
payload: { pull_request }
|
||||||
|
};
|
||||||
|
}
|
||||||
|
|
||||||
|
// A GitHub API mock exposing only rest.pulls.get, with a call counter so
|
||||||
|
// tests can assert whether the API fallback was actually invoked.
|
||||||
|
function makeStackGithub({ stack = null, error = null } = {}) {
|
||||||
|
const state = { calls: 0 };
|
||||||
|
const github = {
|
||||||
|
rest: {
|
||||||
|
pulls: {
|
||||||
|
get: async () => {
|
||||||
|
state.calls++;
|
||||||
|
if (error) throw error;
|
||||||
|
return { data: { stack } };
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
};
|
||||||
|
return { github, state };
|
||||||
|
}
|
||||||
|
|
||||||
|
const STACK_INFO = { base: { ref: 'dev' }, id: 71540, number: 17978, position: 3, size: 3 };
|
||||||
|
|
||||||
|
describe('detectMergeBranch', () => {
|
||||||
|
it('base ref release adds merging-to-release only and never checks the stack', async () => {
|
||||||
|
const { github, state } = makeStackGithub({ stack: STACK_INFO });
|
||||||
|
const context = makeMergeContext('release', { stack: STACK_INFO });
|
||||||
|
const labels = await detectMergeBranch(github, context);
|
||||||
|
assert.deepEqual(Array.from(labels).sort(), ['merging-to-release']);
|
||||||
|
assert.equal(state.calls, 0);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('base ref beta adds merging-to-beta only and never checks the stack', async () => {
|
||||||
|
const { github, state } = makeStackGithub({ stack: STACK_INFO });
|
||||||
|
const context = makeMergeContext('beta', { stack: STACK_INFO });
|
||||||
|
const labels = await detectMergeBranch(github, context);
|
||||||
|
assert.deepEqual(Array.from(labels).sort(), ['merging-to-beta']);
|
||||||
|
assert.equal(state.calls, 0);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('stack present on the webhook payload adds stacked-pr without calling the API', async () => {
|
||||||
|
const { github, state } = makeStackGithub();
|
||||||
|
const context = makeMergeContext('feature-branch', { stack: STACK_INFO });
|
||||||
|
const labels = await detectMergeBranch(github, context);
|
||||||
|
assert.deepEqual(Array.from(labels).sort(), ['stacked-pr']);
|
||||||
|
assert.equal(state.calls, 0);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('stack absent from payload falls back to the API and adds stacked-pr', async () => {
|
||||||
|
const { github, state } = makeStackGithub({ stack: STACK_INFO });
|
||||||
|
const context = makeMergeContext('feature-branch');
|
||||||
|
const labels = await detectMergeBranch(github, context);
|
||||||
|
assert.deepEqual(Array.from(labels).sort(), ['stacked-pr']);
|
||||||
|
assert.equal(state.calls, 1);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('bottom of a stack (base ref dev, stack present) still adds stacked-pr', async () => {
|
||||||
|
const { github, state } = makeStackGithub();
|
||||||
|
const context = makeMergeContext('dev', { stack: STACK_INFO });
|
||||||
|
const labels = await detectMergeBranch(github, context);
|
||||||
|
assert.deepEqual(Array.from(labels).sort(), ['stacked-pr']);
|
||||||
|
assert.equal(state.calls, 0);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('not stacked, base ref not dev adds chained-pr', async () => {
|
||||||
|
const { github } = makeStackGithub({ stack: null });
|
||||||
|
const context = makeMergeContext('feature-branch');
|
||||||
|
const labels = await detectMergeBranch(github, context);
|
||||||
|
assert.deepEqual(Array.from(labels).sort(), ['chained-pr']);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('not stacked, base ref dev adds no labels', async () => {
|
||||||
|
const { github } = makeStackGithub({ stack: null });
|
||||||
|
const context = makeMergeContext('dev');
|
||||||
|
const labels = await detectMergeBranch(github, context);
|
||||||
|
assert.deepEqual(Array.from(labels).sort(), []);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('a failed stack lookup falls back to not-stacked, so a feature-branch base adds chained-pr', async () => {
|
||||||
|
const { github, state } = makeStackGithub({ error: new Error('API unavailable') });
|
||||||
|
const context = makeMergeContext('feature-branch');
|
||||||
|
const labels = await detectMergeBranch(github, context);
|
||||||
|
assert.deepEqual(Array.from(labels).sort(), ['chained-pr']);
|
||||||
|
assert.equal(state.calls, 1);
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
// ---------------------------------------------------------------------------
|
// ---------------------------------------------------------------------------
|
||||||
// detectNewPlatforms
|
// detectNewPlatforms
|
||||||
// ---------------------------------------------------------------------------
|
// ---------------------------------------------------------------------------
|
||||||
|
|||||||
@@ -35,7 +35,7 @@ jobs:
|
|||||||
# Scope the minted App token to the minimum needed by auto-label-pr/*.js.
|
# Scope the minted App token to the minimum needed by auto-label-pr/*.js.
|
||||||
permission-contents: read # repos.getContent for CODEOWNERS and file lookups in detectors.js
|
permission-contents: read # repos.getContent for CODEOWNERS and file lookups in detectors.js
|
||||||
permission-issues: write # listLabelsOnIssue, addLabels, removeLabel, list/createComment
|
permission-issues: write # listLabelsOnIssue, addLabels, removeLabel, list/createComment
|
||||||
permission-pull-requests: write # pulls.listFiles, list/create/update/dismissReview
|
permission-pull-requests: write # pulls.get, pulls.listFiles, list/create/update/dismissReview
|
||||||
|
|
||||||
- name: Auto Label PR
|
- name: Auto Label PR
|
||||||
uses: actions/github-script@3a2844b7e9c422d3c10d287c895573f7108da1b3 # v9.0.0
|
uses: actions/github-script@3a2844b7e9c422d3c10d287c895573f7108da1b3 # v9.0.0
|
||||||
|
|||||||
Reference in New Issue
Block a user