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

Add sub-action to merge artifacts #505

Merged
merged 6 commits into from Jan 23, 2024
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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
91 changes: 86 additions & 5 deletions .github/workflows/test.yml
robherley marked this conversation as resolved.
Show resolved Hide resolved
Expand Up @@ -141,12 +141,16 @@ jobs:
}
shell: pwsh

- name: 'Alter file 1 content'
run: |
echo "This file has changed" > path/to/dir-1/file1.txt

# Replace the contents of Artifact #1
- name: 'Overwrite artifact #1 again'
- name: 'Overwrite artifact #1'
uses: ./
with:
name: 'Artifact-A-${{ matrix.runs-on }}'
path: path/to/dir-2/file2.txt
path: path/to/dir-1/file1.txt
overwrite: true

# Download replaced Artifact #1 and verify the correctness of the content
Expand All @@ -158,13 +162,90 @@ jobs:

- name: 'Verify Artifact #1 again'
run: |
$file = "overwrite/some/new/path/file2.txt"
$file = "overwrite/some/new/path/file1.txt"
if(!(Test-Path -path $file))
{
Write-Error "Expected file does not exist"
}
if(!((Get-Content $file) -ceq "Hello world from file #2"))
if(!((Get-Content $file) -ceq "This file has changed"))
{
Write-Error "File contents of downloaded artifacts are incorrect"
Write-Error "File contents of downloaded artifact are incorrect"
}
shell: pwsh
merge:
name: Merge
needs: build
runs-on: ubuntu-latest

steps:
- name: Checkout
uses: actions/checkout@v4

# Merge all artifacts from previous jobs
- name: Merge all artifacts in run
uses: ./merge/
with:
# our matrix produces artifacts with the same file, this prevents "stomping" on each other, also makes it
# easier to identify each of the merged artifacts
separate-directories: true
- name: 'Download merged artifacts'
uses: actions/download-artifact@v4
with:
name: merged-artifacts
path: all-merged-artifacts
- name: 'Check merged artifact has directories for each artifact'
run: |
$artifacts = @(
"Artifact-A-ubuntu-latest",
"Artifact-A-macos-latest",
"Artifact-A-windows-latest",
"Artifact-Wildcard-ubuntu-latest",
"Artifact-Wildcard-macos-latest",
"Artifact-Wildcard-windows-latest",
"Multi-Path-Artifact-ubuntu-latest",
"Multi-Path-Artifact-macos-latest",
"Multi-Path-Artifact-windows-latest"
)

foreach ($artifact in $artifacts) {
$path = "all-merged-artifacts/$artifact"
if (!(Test-Path $path)) {
Write-Error "$path does not exist."
}
}
shell: pwsh

# Merge Artifact-A-* from previous jobs
- name: Merge all Artifact-A
uses: ./merge/
with:
name: Merged-Artifact-As
pattern: 'Artifact-A-*'
separate-directories: true

# Download merged artifacts and verify the correctness of the content
- name: 'Download merged artifacts'
uses: actions/download-artifact@v4
with:
name: Merged-Artifact-As
path: merged-artifact-a

- name: 'Verify merged artifacts'
run: |
$files = @(
"merged-artifact-a/Artifact-A-ubuntu-latest/file1.txt",
"merged-artifact-a/Artifact-A-macos-latest/file1.txt",
"merged-artifact-a/Artifact-A-windows-latest/file1.txt"
)

foreach ($file in $files) {
if (!(Test-Path $file)) {
Write-Error "$file does not exist."
}

if (!((Get-Content $file) -ceq "This file has changed")) {
Write-Error "$file has incorrect content."
}
}
shell: pwsh

26 changes: 26 additions & 0 deletions .licenses/npm/minimatch.dep.yml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 2 additions & 0 deletions README.md
Expand Up @@ -41,6 +41,8 @@ The release of upload-artifact@v4 and download-artifact@v4 are major changes to

For more information, see the [`@actions/artifact`](https://github.com/actions/toolkit/tree/main/packages/artifact) documentation.

There is also a new sub-action, `actions/upload-artifact/merge`. For more info, check out that action's [README](./merge/README.md).

### Improvements

1. Uploads are significantly faster, upwards of 90% improvement in worst case scenarios.
Expand Down
175 changes: 175 additions & 0 deletions __tests__/merge.test.ts
@@ -0,0 +1,175 @@
import * as core from '@actions/core'
import artifact from '@actions/artifact'
import {run} from '../src/merge/merge-artifacts'
import {Inputs} from '../src/merge/constants'
import * as search from '../src/shared/search'

const fixtures = {
artifactName: 'my-merged-artifact',
tmpDirectory: '/tmp/merge-artifact',
filesToUpload: [
'/some/artifact/path/file-a.txt',
'/some/artifact/path/file-b.txt',
'/some/artifact/path/file-c.txt'
],
artifacts: [
{
name: 'my-artifact-a',
id: 1,
size: 100,
createdAt: new Date('2024-01-01T00:00:00Z')
},
{
name: 'my-artifact-b',
id: 2,
size: 100,
createdAt: new Date('2024-01-01T00:00:00Z')
},
{
name: 'my-artifact-c',
id: 3,
size: 100,
createdAt: new Date('2024-01-01T00:00:00Z')
}
]
}

jest.mock('@actions/github', () => ({
context: {
repo: {
owner: 'actions',
repo: 'toolkit'
},
runId: 123,
serverUrl: 'https://github.com'
}
}))

jest.mock('@actions/core')

jest.mock('fs/promises', () => ({
mkdtemp: jest.fn().mockResolvedValue('/tmp/merge-artifact'),
rm: jest.fn().mockResolvedValue(undefined)
}))

/* eslint-disable no-unused-vars */
const mockInputs = (overrides?: Partial<{[K in Inputs]?: any}>) => {
const inputs = {
[Inputs.Name]: 'my-merged-artifact',
[Inputs.Pattern]: '*',
[Inputs.SeparateDirectories]: false,
[Inputs.RetentionDays]: 0,
[Inputs.CompressionLevel]: 6,
[Inputs.DeleteMerged]: false,
...overrides
}

;(core.getInput as jest.Mock).mockImplementation((name: string) => {
return inputs[name]
})
;(core.getBooleanInput as jest.Mock).mockImplementation((name: string) => {
return inputs[name]
})

return inputs
}

describe('merge', () => {
beforeEach(async () => {
mockInputs()

jest
.spyOn(artifact, 'listArtifacts')
.mockResolvedValue({artifacts: fixtures.artifacts})

jest.spyOn(artifact, 'downloadArtifact').mockResolvedValue({
downloadPath: fixtures.tmpDirectory
})

jest.spyOn(search, 'findFilesToUpload').mockResolvedValue({
filesToUpload: fixtures.filesToUpload,
rootDirectory: fixtures.tmpDirectory
})

jest.spyOn(artifact, 'uploadArtifact').mockResolvedValue({
size: 123,
id: 1337
})

jest
.spyOn(artifact, 'deleteArtifact')
.mockImplementation(async artifactName => {
const artifact = fixtures.artifacts.find(a => a.name === artifactName)
if (!artifact) throw new Error(`Artifact ${artifactName} not found`)
return {id: artifact.id}
})
})

it('merges artifacts', async () => {
await run()

for (const a of fixtures.artifacts) {
expect(artifact.downloadArtifact).toHaveBeenCalledWith(a.id, {
path: fixtures.tmpDirectory
})
}

expect(artifact.uploadArtifact).toHaveBeenCalledWith(
fixtures.artifactName,
fixtures.filesToUpload,
fixtures.tmpDirectory,
{compressionLevel: 6}
)
})

it('fails if no artifacts found', async () => {
mockInputs({[Inputs.Pattern]: 'this-does-not-match'})

expect(run()).rejects.toThrow()

expect(artifact.uploadArtifact).not.toBeCalled()
expect(artifact.downloadArtifact).not.toBeCalled()
})

it('supports custom compression level', async () => {
mockInputs({
[Inputs.CompressionLevel]: 2
})

await run()

expect(artifact.uploadArtifact).toHaveBeenCalledWith(
fixtures.artifactName,
fixtures.filesToUpload,
fixtures.tmpDirectory,
{compressionLevel: 2}
)
})

it('supports custom retention days', async () => {
mockInputs({
[Inputs.RetentionDays]: 7
})

await run()

expect(artifact.uploadArtifact).toHaveBeenCalledWith(
fixtures.artifactName,
fixtures.filesToUpload,
fixtures.tmpDirectory,
{retentionDays: 7, compressionLevel: 6}
)
})

it('supports deleting artifacts after merge', async () => {
mockInputs({
[Inputs.DeleteMerged]: true
})

await run()

for (const a of fixtures.artifacts) {
expect(artifact.deleteArtifact).toHaveBeenCalledWith(a.name)
}
})
})