Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

switch from github workflows to github actions workflows; test [githubactionsworkflowstatus githubworkflowstatus] #8475

Merged
merged 5 commits into from
Dec 15, 2022
Merged
Show file tree
Hide file tree
Changes from all 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
101 changes: 101 additions & 0 deletions services/github/github-actions-workflow-status.service.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,101 @@
import Joi from 'joi'
import { isBuildStatus, renderBuildStatusBadge } from '../build-status.js'
import { NotFound } from '../index.js'
import { GithubAuthV3Service } from './github-auth-service.js'
import { documentation, errorMessagesFor } from './github-helpers.js'

const schema = Joi.object({
workflow_runs: Joi.array()
.items(
Joi.object({
conclusion: Joi.alternatives()
.try(isBuildStatus, Joi.equal('no status'))
.required(),
})
)
.required()
.min(0)
.max(1),
}).required()

const queryParamSchema = Joi.object({
event: Joi.string(),
branch: Joi.string().required(),
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've made branch a required param for this badge. Not my favourite solution, but workflow can contain a / and branch=HEAD doesn't work so we can't make the default "whatever the default branch is" in one request, so required query param it is. Lets lay that marker down while we're making a breaking change.

I don't think "latest workflow status across all branches" is a sensible default.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you know off hand if this has to explicitly be a branch or will the upstream endpoint accept any valid ref?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The docs say

branch string

Returns workflow runs associated with a branch. Use the name of the branch of the push.

Anecdotally, I tried some calls with a tag and a commit hash (which I expected would return a valid result if this could accept any ref). They returned 0 results.

}).required()

const keywords = ['action', 'actions']

export default class GithubActionsWorkflowStatus extends GithubAuthV3Service {
static category = 'build'

static route = {
base: 'github/actions/workflow/status',
pattern: ':user/:repo/:workflow+',
chris48s marked this conversation as resolved.
Show resolved Hide resolved
queryParamSchema,
}

static examples = [
{
title: 'GitHub Workflow Status',
namedParams: {
user: 'actions',
repo: 'toolkit',
workflow: 'unit-tests.yml',
},
queryParams: {
branch: 'main',
},
staticPreview: renderBuildStatusBadge({
status: 'passing',
}),
documentation,
keywords,
},
{
title: 'GitHub Workflow Status (with event)',
namedParams: {
user: 'actions',
repo: 'toolkit',
workflow: 'unit-tests.yml',
},
queryParams: {
event: 'push',
branch: 'main',
},
staticPreview: renderBuildStatusBadge({
status: 'passing',
}),
documentation,
keywords,
},
]

static defaultBadgeData = {
label: 'build',
}

async fetch({ user, repo, workflow, branch, event }) {
return await this._requestJson({
schema,
url: `/repos/${user}/${repo}/actions/workflows/${workflow}/runs`,
options: {
searchParams: {
branch,
event,
page: '1',
per_page: '1',
exclude_pull_requests: 'true',
Comment on lines +85 to +87
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These params are here for performance reasons. There's huge execution time difference between this query with exclude_pull_requests=true (fast) vs exclude_pull_requests=false (slow)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Interesting. Definitely understand the performance angle, but curious if there's any functional impacts? Am I correctly interpreting this as meaning that runs triggered by a pull request would be excluded?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nope. Passing this means the pull_requests array in the JSON response is not populated, but we aren't using any values from it so it doesn't really matter here. We can still get the details of a workflow that is triggered by a PR. For example, if I call https://api.github.com/repos/badges/shields/actions/workflows/enforce-dependency-review.yml/runs?branch=frontend-gha&exclude_pull_requests=true (the enforce-dependency-review.yml workflow on our repo only runs on PR triggers), we get a response with all the info we need for a badge. Just the pull_requests array is empty.

},
},
errorMessages: errorMessagesFor('repo or workflow not found'),
})
}

async handle({ user, repo, workflow }, { branch, event }) {
const data = await this.fetch({ user, repo, workflow, branch, event })
if (data.workflow_runs.length === 0) {
throw new NotFound({ prettyMessage: 'branch or event not found' })
}
return renderBuildStatusBadge({ status: data.workflow_runs[0].conclusion })
}
}
59 changes: 59 additions & 0 deletions services/github/github-actions-workflow-status.tester.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
import Joi from 'joi'
import { isBuildStatus } from '../build-status.js'
import { createServiceTester } from '../tester.js'
export const t = await createServiceTester()

const isWorkflowStatus = Joi.alternatives()
.try(isBuildStatus, Joi.equal('no status'))
.required()

t.create('missing branch param')
.get('/actions/toolkit/unit-tests.yml.json')
.expectBadge({
label: 'build',
message: 'invalid query parameter: branch',
})

t.create('nonexistent repo')
.get('/badges/shields-fakeness/fake.yml.json?branch=main')
.expectBadge({
label: 'build',
message: 'repo or workflow not found',
})

t.create('nonexistent workflow')
.get('/actions/toolkit/not-a-real-workflow.yml.json?branch=main')
.expectBadge({
label: 'build',
message: 'repo or workflow not found',
})

t.create('nonexistent branch')
.get('/actions/toolkit/unit-tests.yml.json?branch=not-a-real-branch')
.expectBadge({
label: 'build',
message: 'branch or event not found',
})

t.create('nonexistent event')
.get(
'/actions/toolkit/unit-tests.yml.json?branch=main&event=not-a-real-event'
)
.expectBadge({
label: 'build',
message: 'branch or event not found',
})

t.create('valid workflow')
.get('/actions/toolkit/unit-tests.yml.json?branch=main')
.expectBadge({
label: 'build',
message: isWorkflowStatus,
})

t.create('valid workflow (with event)')
.get('/actions/toolkit/unit-tests.yml.json?branch=main&event=push')
.expectBadge({
label: 'build',
message: isWorkflowStatus,
})
110 changes: 19 additions & 91 deletions services/github/github-workflow-status.service.js
Original file line number Diff line number Diff line change
@@ -1,100 +1,28 @@
import Joi from 'joi'
import { isBuildStatus, renderBuildStatusBadge } from '../build-status.js'
import { BaseSvgScrapingService } from '../index.js'
import { documentation } from './github-helpers.js'
import { BaseService } from '../index.js'

const schema = Joi.object({
message: Joi.alternatives()
.try(isBuildStatus, Joi.equal('no status'))
.required(),
}).required()

const queryParamSchema = Joi.object({
event: Joi.string(),
}).required()

const keywords = ['action', 'actions']

export default class GithubWorkflowStatus extends BaseSvgScrapingService {
export default class DeprecatedGithubWorkflowStatus extends BaseService {
static category = 'build'

static route = {
base: 'github/workflow/status',
pattern: ':user/:repo/:workflow/:branch*',
queryParamSchema,
}

static examples = [
{
title: 'GitHub Workflow Status',
pattern: ':user/:repo/:workflow',
namedParams: {
user: 'actions',
repo: 'toolkit',
workflow: 'toolkit-unit-tests',
},
staticPreview: renderBuildStatusBadge({
status: 'passing',
}),
documentation,
keywords,
},
{
title: 'GitHub Workflow Status (branch)',
pattern: ':user/:repo/:workflow/:branch',
namedParams: {
user: 'actions',
repo: 'toolkit',
workflow: 'toolkit-unit-tests',
branch: 'master',
},
staticPreview: renderBuildStatusBadge({
status: 'passing',
}),
documentation,
keywords,
},
{
title: 'GitHub Workflow Status (event)',
pattern: ':user/:repo/:workflow',
namedParams: {
user: 'actions',
repo: 'toolkit',
workflow: 'toolkit-unit-tests',
},
queryParams: {
event: 'push',
},
staticPreview: renderBuildStatusBadge({
status: 'passing',
}),
documentation,
keywords,
},
]

static defaultBadgeData = {
label: 'build',
}

async fetch({ user, repo, workflow, branch, event }) {
const { message: status } = await this._requestSvg({
schema,
url: `https://github.com/${user}/${repo}/workflows/${encodeURIComponent(
workflow
)}/badge.svg`,
options: { searchParams: { branch, event } },
valueMatcher: />([^<>]+)<\/tspan><\/text><\/g><path/,
errorMessages: {
404: 'repo, branch, or workflow not found',
},
})

return { status }
pattern: ':various+',
}

async handle({ user, repo, workflow, branch }, { event }) {
const { status } = await this.fetch({ user, repo, workflow, branch, event })
return renderBuildStatusBadge({ status })
static examples = []

static defaultBadgeData = { label: 'build' }

async handle() {
return {
label: 'build',
message: 'https://github.com/badges/shields/issues/8671',
/*
This is a 'special' deprecation because we are making a breaking change
We've implemented it as a custom class instead of a normal
deprecatedService so that we can include link.
*/
link: ['https://github.com/badges/shields/issues/8671'],
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because deprecatedService() throws an exception, we can't customise it beyond the label/message so I've done this as a custom class. I think the number of cases where this will be actually clickable will be low but it is worth doing IMO. This is a sufficiently exceptional edge-case that I don't mind this being a bit special and wierd and custom.

color: 'red',
}
}
}
33 changes: 16 additions & 17 deletions services/github/github-workflow-status.tester.js
Original file line number Diff line number Diff line change
@@ -1,43 +1,42 @@
import Joi from 'joi'
import { isBuildStatus } from '../build-status.js'
import { createServiceTester } from '../tester.js'
export const t = await createServiceTester()
import { ServiceTester } from '../tester.js'

const isWorkflowStatus = Joi.alternatives()
.try(isBuildStatus, Joi.equal('no status'))
.required()
export const t = new ServiceTester({
id: 'GithubWorkflowStatus',
title: 'Github Workflow Status',
pathPrefix: '/github/workflow/status',
})

t.create('nonexistent repo')
t.create('no longer available (previously nonexistent repo)')
.get('/badges/shields-fakeness/fake.json')
.expectBadge({
label: 'build',
message: 'repo, branch, or workflow not found',
message: 'https://github.com/badges/shields/issues/8671',
})

t.create('nonexistent workflow')
t.create('no longer available (previously nonexistent workflow)')
.get('/actions/toolkit/not-a-real-workflow.json')
.expectBadge({
label: 'build',
message: 'repo, branch, or workflow not found',
message: 'https://github.com/badges/shields/issues/8671',
})

t.create('valid workflow')
t.create('no longer available (previously valid workflow)')
.get('/actions/toolkit/toolkit-unit-tests.json')
.expectBadge({
label: 'build',
message: isWorkflowStatus,
message: 'https://github.com/badges/shields/issues/8671',
})

t.create('valid workflow (branch)')
t.create('no longer available (previously valid workflow - branch)')
.get('/actions/toolkit/toolkit-unit-tests/master.json')
.expectBadge({
label: 'build',
message: isWorkflowStatus,
message: 'https://github.com/badges/shields/issues/8671',
})

t.create('valid workflow (event)')
t.create('no longer available (previously valid workflow - event)')
.get('/actions/toolkit/toolkit-unit-tests.json?event=push')
.expectBadge({
label: 'build',
message: isWorkflowStatus,
message: 'https://github.com/badges/shields/issues/8671',
})