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.

Sign up for GitHub

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

Jump to bottom

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

Merged
merged 5 commits into from
Dec 15, 2022
Merged

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

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
808dfec
switch from github workflows to github actions workflows
chris48s Oct 3, 2022
7ea7cf5
update github actions workflow badge to use api
chris48s Oct 17, 2022
de39617
add test case for missing branch param
chris48s Oct 17, 2022
e0b7dbd
custom deprecation message
chris48s Dec 1, 2022
7e7d006
Merge branch 'master' into 8146-gitlab-workflows
chris48s Dec 15, 2022
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
85 changes: 85 additions & 0 deletions services/github/github-actions-workflow-status.service.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,85 @@
import Joi from 'joi'
import { isBuildStatus, renderBuildStatusBadge } from '../build-status.js'
import { BaseSvgScrapingService } from '../index.js'
import { documentation } from './github-helpers.js'

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

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

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

export default class GithubActionsWorkflowStatus extends BaseSvgScrapingService {
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',
},
staticPreview: renderBuildStatusBadge({
status: 'passing',
}),
documentation,
keywords,
},
{
title: 'GitHub Workflow Status (branch and 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 }) {
const { message: status } = await this._requestSvg({
schema,
url: `https://github.com/${user}/${repo}/actions/workflows/${encodeURIComponent(
workflow
)}/badge.svg`,
options: { searchParams: { branch, event } },
valueMatcher: />([^<>]+)<\/tspan><\/text><\/g><path/,
errorMessages: {
404: 'repo, branch, or workflow not found',
},
})

return { status }
}

async handle({ user, repo, workflow }, { branch, event }) {
const { status } = await this.fetch({ user, repo, workflow, branch, event })
return renderBuildStatusBadge({ status })
}
}
43 changes: 43 additions & 0 deletions services/github/github-actions-workflow-status.tester.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
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('nonexistent repo')
.get('/badges/shields-fakeness/fake.yml.json')
.expectBadge({
label: 'build',
message: 'repo, branch, or workflow not found',
})

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

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

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

t.create('valid workflow (event)')
.get('/actions/toolkit/unit-tests.yml.json?event=push')
.expectBadge({
label: 'build',
message: isWorkflowStatus,
})
107 changes: 9 additions & 98 deletions services/github/github-workflow-status.service.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,100 +1,11 @@
import Joi from 'joi'
import { isBuildStatus, renderBuildStatusBadge } from '../build-status.js'
import { BaseSvgScrapingService } from '../index.js'
import { documentation } from './github-helpers.js'
import { deprecatedService } 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 {
static category = 'build'

static route = {
export default deprecatedService({
Copy link
Member Author

Choose a reason for hiding this comment

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

Normally when we deprecate a badge we are just getting rid of it completely. Often the upstream service has been retired, or whatever. If we can provide a migration path from old to new, we use a redirect. In this case, we can't do either of those things because the :workflow param is not compatible between the two badges. Additionally, the GitHub workflow status badge is one of our most popular badges 😬 I think deprecating the old route and making people switch is the right thing to do because the behaviour of the existing badge is unexpected but its going to generate some support requests/anger/confusion/etc. Before we merge this I definitely want to write up a pinned issue (I've not written it yet, but I will) explaining what is going on and how to migrate.

Because we've never had this situation where we're asking people to switch and we can't provide a redirect, we've never done this before, but I wonder if instead of serving build | no longer available we make the issue and serve build | github.com/badges/shields/issues/9999 (also use the link property for any <object> embeds) to help direct people towards upgrade advice. I'm somewhat conflicted as it is mainly info for maintainers and most of the people who will see it are going to be end users who can't do anything about it.

Copy link
Member

@calebcartwright calebcartwright Nov 30, 2022 •
edited by chris48s
Loading

Choose a reason for hiding this comment

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

but I wonder if instead of serving build | no longer available we make the issue and serve build | github.com//issues/9999 (also use the link property for any <object> embeds) to help direct people towards upgrade advice. I'm somewhat conflicted as it is mainly info for maintainers and most of the people who will see it are going to be end users who can't do anything about it.

I'd recommend going with your issue link suggestion. Completely agreed that 99% of those that see the badge won't be able to do anything about it, but having a no longer available message instead doesn't really improve their experience one way or another and I think having the issue link only increases awareness; some subset of those users that see the issue-link badge in someone else's project will have their own such badges in their respective repos that they do need to take action on but may still be unaware

Copy link
Member Author

Choose a reason for hiding this comment

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

OK. I have made a placeholder issue #8671 so we have a known URL we can link to, and I've done that in de39617 . This should allow us to approve the code changes. I will work on writing up the issue explaining why we are making the change, what users need to do about it, etc before I deploy it.

category: 'build',
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 }
}

async handle({ user, repo, workflow, branch }, { event }) {
const { status } = await this.fetch({ user, repo, workflow, branch, event })
return renderBuildStatusBadge({ status })
}
}
pattern: ':various+',
},
label: 'build',
dateAdded: new Date('2022-10-03'),
})
33 changes: 16 additions & 17 deletions services/github/github-workflow-status.tester.js
View file
Open in desktop
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: 'no longer available',
})

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: 'no longer available',
})

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: 'no longer available',
})

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: 'no longer available',
})

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: 'no longer available',
})