Skip to content

Commit

Permalink
Merge pull request #351 from smartlyio/VULCAN-4165
Browse files Browse the repository at this point in the history
Support `on: push` to simplify workflows
  • Loading branch information
anttiharju authored Jun 18, 2024
2 parents 52dced1 + 29d3266 commit 2226656
Show file tree
Hide file tree
Showing 11 changed files with 317 additions and 315 deletions.
1 change: 0 additions & 1 deletion .github/workflows/check_release_label.yml
Original file line number Diff line number Diff line change
Expand Up @@ -17,4 +17,3 @@ jobs:
- uses: smartlyio/check-versioning-action@v5
with:
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}

Original file line number Diff line number Diff line change
@@ -1,16 +1,13 @@
name: Build and release the action
name: Master Push

# smartlyio/find-changes-action@v1 (under Release flow) only works on pull_request events is the reason for not using "on: push: branches: master"
on:
pull_request:
branches: [master]
types: [closed]
push:
branches:
- master

jobs:
build:
name: Build
# Only run when PR is closed AND merged, not when PR is just closed.
if: github.event.pull_request.merged
uses: ./.github/workflows/build.yml
secrets: inherit

Expand Down
4 changes: 1 addition & 3 deletions .github/workflows/pull_request.yml
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
name: "Pull Request"

on:
pull_request:
branches: [master]
on: pull_request

jobs:
build:
Expand Down
74 changes: 23 additions & 51 deletions __tests__/main.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,76 +39,55 @@ afterEach(() => {
describe('get branch point', () => {
test('missing GITHUB_EVENT_NAME', async () => {
delete process.env['GITHUB_EVENT_NAME']
const context: Context = {
fromOriginalBranchPoint: false,
directoryContaining: null, // Not used by getChangedDirectories
directoryLevels: null,
exclude: /^\.github\/.*/ // Not used by getChangedDirectories
}
await expect(getBranchPoint(context)).rejects.toThrow(
/only works on pull_request/
)
await expect(getBranchPoint()).rejects.toThrow(/only works on pull_request/)
})

test('wrong GITHUB_EVENT_NAME', async () => {
process.env['GITHUB_EVENT_NAME'] = 'push'
const context: Context = {
fromOriginalBranchPoint: false,
directoryContaining: null, // Not used by getChangedDirectories
directoryLevels: null,
exclude: /^\.github\/.*/ // Not used by getChangedDirectories
}
await expect(getBranchPoint(context)).rejects.toThrow(
/only works on pull_request/
)
process.env['GITHUB_EVENT_NAME'] = 'schedule'

await expect(getBranchPoint()).rejects.toThrow(/only works on pull_request/)
})

test('missing GITHUB_EVENT_PATH', async () => {
process.env['GITHUB_EVENT_NAME'] = 'pull_request'
delete process.env['GITHUB_EVENT_PATH']
const context: Context = {
fromOriginalBranchPoint: false,
directoryContaining: null, // Not used by getChangedDirectories
directoryLevels: null,
exclude: /^\.github\/.*/ // Not used by getChangedDirectories
}
await expect(getBranchPoint(context)).rejects.toThrow(
await expect(getBranchPoint()).rejects.toThrow(
/Could not find event payload/
)
})

test('pull request event type closed', async () => {
const eventPath = path.join(__dirname, 'pr_event_closed.json')
const eventData = await fsReal.readFile(eventPath)
const event = JSON.parse(eventData.toString())
process.env['GITHUB_EVENT_NAME'] = 'pull_request'
process.env['GITHUB_EVENT_PATH'] = eventPath

await expect(getBranchPoint()).rejects.toThrow(
/Running find-changes on: pull_request: closed is not supported in v2 - please migrate workflow to on: push:/
)
})

test('get sha of main merged to PR', async () => {
const eventPath = path.join(__dirname, 'pr_event.json')
const eventData = await fsReal.readFile(eventPath)
const event = JSON.parse(eventData.toString())
process.env['GITHUB_EVENT_NAME'] = 'pull_request'
process.env['GITHUB_EVENT_PATH'] = eventPath

const context: Context = {
fromOriginalBranchPoint: false,
directoryContaining: null, // Not used by getChangedDirectories
directoryLevels: null,
exclude: /^\.github\/.*/ // Not used by getChangedDirectories
}
const branchPoint = await getBranchPoint(context)
const branchPoint = await getBranchPoint()
expect(branchPoint).toEqual('origin/master')
})

test('from original branch point', async () => {
const eventPath = path.join(__dirname, 'pr_event.json')
test('on push', async () => {
const eventPath = path.join(__dirname, 'push_event.json')
const eventData = await fsReal.readFile(eventPath)
const event = JSON.parse(eventData.toString())
process.env['GITHUB_EVENT_NAME'] = 'pull_request'
process.env['GITHUB_EVENT_NAME'] = 'push'
process.env['GITHUB_EVENT_PATH'] = eventPath

const context: Context = {
fromOriginalBranchPoint: true,
directoryContaining: null, // Not used by getChangedDirectories
directoryLevels: null,
exclude: /^\.github\/.*/ // Not used by getChangedDirectories
}
const branchPoint = await getBranchPoint(context)
expect(branchPoint).toEqual(event.pull_request.base.sha)
const branchPoint = await getBranchPoint()
expect(branchPoint).toEqual(event.before)
})
})

Expand All @@ -129,7 +108,6 @@ deeply/nested/package/README
top-level-file
`
const context: Context = {
fromOriginalBranchPoint: false,
directoryContaining: null, // Not used by getChangedDirectories
directoryLevels: null,
exclude: /^\.github\/.*/ // Not used by getChangedDirectories
Expand Down Expand Up @@ -161,7 +139,6 @@ deeply/nested/package/README
top-level-file
`
const context: Context = {
fromOriginalBranchPoint: false,
directoryContaining: null,
directoryLevels: 1,
exclude: /^\.github\/.*/ // Not used by getChangedDirectories
Expand Down Expand Up @@ -193,7 +170,6 @@ deeply/nested/package/README
top-level-file
`
const context: Context = {
fromOriginalBranchPoint: false,
directoryContaining: null,
directoryLevels: 2,
exclude: /^\.github\/.*/ // Not used by getChangedDirectories
Expand Down Expand Up @@ -228,7 +204,6 @@ deeply/nested/package/README
top-level-file
`
const context: Context = {
fromOriginalBranchPoint: false,
directoryContaining: null, // Not used by getChangedDirectories
directoryLevels: null,
exclude: /^\.github\/.*/ // Not used by getChangedDirectories
Expand Down Expand Up @@ -289,7 +264,6 @@ describe('filterGitOutputByFile', () => {
]
const expected = ['package1', 'package2', 'nested', 'deeply']
const context: Context = {
fromOriginalBranchPoint: false,
directoryContaining: null,
directoryLevels: null,
exclude: /^\.github($|\/.*)/
Expand All @@ -307,7 +281,6 @@ describe('filterGitOutputByFile', () => {
]
const expected = ['nested/package', 'deeply/nested']
const context: Context = {
fromOriginalBranchPoint: false,
directoryContaining: null,
directoryLevels: null,
exclude: /^\.github($|\/.*)/
Expand All @@ -321,7 +294,6 @@ describe('filterGitOutputByFile', () => {
const changedDirectories = ['__tests__', 'deeply/nested', '.github/actions']
const expected = ['__tests__']
const context: Context = {
fromOriginalBranchPoint: false,
directoryContaining: 'pr_event.json',
directoryLevels: null,
exclude: /^\.github($|\/.*)/
Expand Down
12 changes: 12 additions & 0 deletions __tests__/pr_event_closed.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
{
"repository": {
"default_branch": "master"
},
"action": "closed",
"pull_request": {
"base": {
"sha": "pull_request_sha"
}
}
}

8 changes: 8 additions & 0 deletions __tests__/push_event.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
{
"repository": {
"default_branch": "master"
},
"action": "closed",
"before": "push_sha"
}

8 changes: 0 additions & 8 deletions action.yml
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,6 @@ name: 'Find changed directories'
description: 'Create matrix expression to build only changed packages'
author: 'Smartly.io'
inputs:
from-original-branch-point:
description: >-
Find the changes from the original branch point.
The default is to find changes relative to the *current* main
branch, after merging the main branch into this PR.
required: false
default: false
directory_containing:
description: >-
Find changes to directories containing a file with this name.
Expand Down
Loading

0 comments on commit 2226656

Please sign in to comment.