Skip to content

Commit d8f578c

Browse files
authored
🐛 GraphQL workflow processing fixes (#129)
1 parent af95e88 commit d8f578c

4 files changed

Lines changed: 162 additions & 109 deletions

File tree

src/github/repository.js

Lines changed: 16 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -281,32 +281,25 @@ export default class Repository extends Base {
281281
const [o, n] = repoName.split('/')
282282

283283
const {
284-
data: {
285-
data: {
286-
repository: {
287-
nwo,
288-
owner: {login: owner},
289-
name,
290-
id,
291-
node_id,
292-
visibility,
293-
isArchived,
294-
isFork,
295-
defaultBranchRef,
296-
},
297-
},
284+
repository: {
285+
nwo,
286+
owner: {login: owner},
287+
name,
288+
id,
289+
node_id,
290+
visibility,
291+
isArchived,
292+
isFork,
293+
defaultBranchRef,
298294
},
299-
} = await this.octokit.request('POST /graphql', {
300-
query: REPO_QUERY,
301-
variables: {owner: o, name: n},
302-
})
295+
} = await this.octokit.graphql(REPO_QUERY, {owner: o, name: n})
303296

304297
this.#nwo = nwo
305298
this.#owner = owner
306299
this.#name = name
307300
this.#repo = {
308-
owner: owner.login,
309-
name: name,
301+
owner,
302+
name,
310303
}
311304
this.#id = id
312305
this.#node_id = node_id
@@ -353,16 +346,9 @@ export default class Repository extends Base {
353346
this.spinner.text = `Loading workflows for repository ${cyan(`${owner}/${repo}`)}...`
354347

355348
try {
356-
const {
357-
data: {
358-
data: {repository},
359-
},
360-
} = await this.octokit.request('POST /graphql', {
361-
query: WORKFLOWS_QUERY,
362-
variables: {
363-
owner,
364-
name: repo,
365-
},
349+
const {repository} = await this.octokit.graphql(WORKFLOWS_QUERY, {
350+
owner,
351+
name: repo,
366352
})
367353

368354
const wfs = []

src/report/report.js

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -653,7 +653,10 @@ export default class Report {
653653
processWorkflow(wf, repo, reportTotalCounts) {
654654
const {language, text, yaml} = wf
655655

656-
if (language !== 'YAML') {
656+
// Robust language check:
657+
// - If a language value is provided and it's not 'YAML', and we also do NOT have a parsed yaml object, skip.
658+
// - If language is null/undefined but we have a yaml object, proceed (some APIs may omit language metadata).
659+
if (language && language !== 'YAML' && !yaml) {
657660
this.#logger.warn(
658661
`Skipping workflow ${wf.path} in repository ${repo.nwo} due to unsupported language: ${language}`,
659662
)

test/github/repository.test.js

Lines changed: 78 additions & 78 deletions
Original file line numberDiff line numberDiff line change
@@ -126,113 +126,112 @@ describe('repository', () => {
126126
*/
127127
describe('workflow operations', () => {
128128
beforeEach(() => {
129-
// Mock the octokit requests
130-
repository.octokit.request = jest.fn()
129+
// Mock the octokit GraphQL client
131130
repository.octokit.graphql = jest.fn()
132131
})
133132

134133
test('should fetch workflows for repository', async () => {
135-
const mockGraphQLResponse = {
136-
data: {
137-
data: {
138-
repository: {
139-
object: {
140-
entries: [
141-
{
142-
name: 'ci.yml',
143-
path: '.github/workflows/ci.yml',
144-
language: {name: 'YAML'},
145-
object: {
146-
text: 'name: CI\non: push',
147-
isTruncated: false,
148-
},
149-
},
150-
{
151-
name: 'release.yml',
152-
path: '.github/workflows/release.yml',
153-
language: {name: 'YAML'},
154-
object: {
155-
text: 'name: Release\non: release',
156-
isTruncated: false,
157-
},
158-
},
159-
],
134+
repository.octokit.graphql.mockResolvedValueOnce({
135+
repository: {
136+
object: {
137+
entries: [
138+
{
139+
name: 'ci.yml',
140+
path: '.github/workflows/ci.yml',
141+
language: {name: 'YAML'},
142+
object: {text: 'name: CI\non: push', isTruncated: false, node_id: '1'},
160143
},
161-
},
144+
{
145+
name: 'release.yml',
146+
path: '.github/workflows/release.yml',
147+
language: {name: 'YAML'},
148+
object: {text: 'name: Release\non: release', isTruncated: false, node_id: '2'},
149+
},
150+
],
162151
},
163152
},
164-
}
153+
})
165154

166-
repository.octokit.request.mockResolvedValue(mockGraphQLResponse)
155+
// Mock the REST workflow detail & runs requests invoked by Workflow#getWorkflow
156+
repository.octokit.request = jest
157+
.fn()
158+
// First workflow detail
159+
.mockResolvedValueOnce({
160+
data: {
161+
id: 101,
162+
node_id: 'W_flow_101',
163+
name: 'CI',
164+
path: '.github/workflows/ci.yml',
165+
state: 'active',
166+
created_at: '2024-01-01T00:00:00Z',
167+
updated_at: '2024-01-02T00:00:00Z',
168+
},
169+
})
170+
// First workflow runs (only most recent run needed)
171+
.mockResolvedValueOnce({
172+
data: {
173+
workflow_runs: [{updated_at: '2024-01-03T00:00:00Z'}],
174+
},
175+
})
176+
// Second workflow detail
177+
.mockResolvedValueOnce({
178+
data: {
179+
id: 202,
180+
node_id: 'W_flow_202',
181+
name: 'Release',
182+
path: '.github/workflows/release.yml',
183+
state: 'active',
184+
created_at: '2024-02-01T00:00:00Z',
185+
updated_at: '2024-02-02T00:00:00Z',
186+
},
187+
})
188+
// Second workflow runs
189+
.mockResolvedValueOnce({
190+
data: {
191+
workflow_runs: [{updated_at: '2024-02-03T00:00:00Z'}],
192+
},
193+
})
167194

168195
const workflows = await repository.getWorkflows('mona', 'sample-repo')
169196

170197
expect(Array.isArray(workflows)).toBe(true)
171-
// Since the actual implementation will depend on Workflow constructor,
172-
// we just verify that the method runs without error
173-
expect(workflows).toBeDefined()
198+
expect(workflows).toHaveLength(2)
174199
})
175200

176201
test('should handle repositories without workflows', async () => {
177-
const mockGraphQLResponse = {
178-
data: {
179-
data: {
180-
repository: {
181-
object: null,
182-
},
183-
},
184-
},
185-
}
186-
187-
repository.octokit.request.mockResolvedValueOnce(mockGraphQLResponse)
202+
repository.octokit.graphql.mockResolvedValueOnce({
203+
repository: {object: null},
204+
})
188205

189206
const workflows = await repository.getWorkflows('mona', 'sample-repo')
190-
191207
expect(Array.isArray(workflows)).toBe(true)
192208
expect(workflows).toHaveLength(0)
193209
})
194210

195211
test('should handle empty workflow directory', async () => {
196-
const mockGraphQLResponse = {
197-
data: {
198-
data: {
199-
repository: {
200-
object: {
201-
entries: [],
202-
},
203-
},
204-
},
205-
},
206-
}
207-
208-
repository.octokit.request.mockResolvedValueOnce(mockGraphQLResponse)
212+
repository.octokit.graphql.mockResolvedValueOnce({
213+
repository: {object: {entries: []}},
214+
})
209215

210216
const workflows = await repository.getWorkflows('mona', 'sample-repo')
211-
212217
expect(Array.isArray(workflows)).toBe(true)
213218
expect(workflows).toHaveLength(0)
214219
})
215220

216221
test('should fetch repository metadata', async () => {
217-
const mockRepoData = {
218-
data: {
219-
data: {
220-
repository: {
221-
nwo: 'mona/sample-repo',
222-
owner: {login: 'mona'},
223-
name: 'sample-repo',
224-
id: 123,
225-
node_id: 'MDEwOlJlcG9zaXRvcnkx',
226-
visibility: 'PUBLIC',
227-
isArchived: false,
228-
isFork: false,
229-
defaultBranchRef: {name: 'main'},
230-
},
231-
},
222+
repository.octokit.graphql.mockResolvedValueOnce({
223+
repository: {
224+
nwo: 'mona/sample-repo',
225+
owner: {login: 'mona'},
226+
name: 'sample-repo',
227+
id: 123,
228+
node_id: 'MDEwOlJlcG9zaXRvcnkx',
229+
visibility: 'PUBLIC',
230+
isArchived: false,
231+
isFork: false,
232+
defaultBranchRef: {name: 'main'},
232233
},
233-
}
234-
235-
repository.octokit.request.mockResolvedValueOnce(mockRepoData)
234+
})
236235

237236
const result = await repository.getRepo('mona/sample-repo')
238237

@@ -242,6 +241,7 @@ describe('repository', () => {
242241
expect(result.visibility).toBe('PUBLIC')
243242
expect(result.isArchived).toBe(false)
244243
expect(result.isFork).toBe(false)
244+
expect(result.branch).toBe('main')
245245
})
246246

247247
test('should handle API errors when fetching workflows', async () => {
@@ -255,7 +255,7 @@ describe('repository', () => {
255255
},
256256
}
257257

258-
repository.octokit.request.mockResolvedValueOnce(mockGraphQLResponse)
258+
repository.octokit.graphql.mockResolvedValueOnce(mockGraphQLResponse)
259259

260260
const workflows = await repository.getWorkflows('mona', 'sample-repo')
261261
expect(Array.isArray(workflows)).toBe(true)

test/report/report-extraction.test.js

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -113,4 +113,68 @@ describe('Report extraction end-to-end (regression)', () => {
113113
expect(wf.vars instanceof Set && wf.vars.size).toBeGreaterThan(0)
114114
expect(wf.uses instanceof Set && wf.uses.size).toBeGreaterThan(0)
115115
})
116+
117+
test('should process workflow when language is null but yaml object exists', async () => {
118+
const flags = {
119+
token: 'TEST',
120+
repository: 'o/r',
121+
csv: null,
122+
json: null,
123+
md: null,
124+
listeners: true,
125+
permissions: true,
126+
runsOn: true,
127+
secrets: true,
128+
vars: true,
129+
uses: true,
130+
unique: 'false',
131+
}
132+
133+
const report = new Report(flags, mockLogger(), mockCache())
134+
135+
const wf = createWorkflow()
136+
wf.language = null // simulate missing language metadata
137+
138+
const data = await report.createReport({workflows: [wf]})
139+
expect(data).toHaveLength(1)
140+
expect(data[0].name).toBe('CI Full')
141+
})
142+
143+
test('should skip workflow when language is non-YAML and yaml object missing', async () => {
144+
const flags = {
145+
token: 'TEST',
146+
repository: 'o/r',
147+
csv: null,
148+
json: null,
149+
md: null,
150+
listeners: true,
151+
permissions: true,
152+
runsOn: true,
153+
secrets: true,
154+
vars: true,
155+
uses: true,
156+
unique: 'false',
157+
}
158+
159+
const report = new Report(flags, mockLogger(), mockCache())
160+
161+
const invalidWf = {
162+
node_id: 'WF_node_invalid',
163+
path: '.github/workflows/script.js',
164+
language: 'JavaScript',
165+
text: 'console.log("hi")',
166+
yaml: null,
167+
state: 'active',
168+
created_at: new Date().toISOString(),
169+
updated_at: new Date().toISOString(),
170+
last_run_at: new Date().toISOString(),
171+
}
172+
173+
const validWf = createWorkflow()
174+
175+
const data = await report.createReport({workflows: [invalidWf, validWf]})
176+
// Only the valid YAML workflow should be processed
177+
expect(data).toHaveLength(1)
178+
expect(data[0].workflow).toBe('.github/workflows/ci.yml')
179+
})
116180
})

0 commit comments

Comments
 (0)