Skip to content

Commit

Permalink
React to feedback
Browse files Browse the repository at this point in the history
  • Loading branch information
Aiqiao Yan authored and Aiqiao Yan committed May 15, 2020
1 parent b3c8e19 commit 4dcea16
Show file tree
Hide file tree
Showing 16 changed files with 59 additions and 73 deletions.
8 changes: 4 additions & 4 deletions .github/workflows/artifact-tests.yml
Expand Up @@ -64,8 +64,8 @@ jobs:
- name: Verify downloadArtifact()
shell: bash
run: |
scripts/test-artifact-file.sh "artifact-1-directory/artifact-path/world.txt" "${{ env.non-gzip-artifact-content }}"
scripts/test-artifact-file.sh "artifact-2-directory/artifact-path/gzip.txt" "${{ env.gzip-artifact-content }}"
packages/artifact/__tests__/test-artifact-file.sh "artifact-1-directory/artifact-path/world.txt" "${{ env.non-gzip-artifact-content }}"
packages/artifact/__tests__/test-artifact-file.sh "artifact-2-directory/artifact-path/gzip.txt" "${{ env.gzip-artifact-content }}"
- name: Download artifacts using downloadAllArtifacts()
run: |
Expand All @@ -75,5 +75,5 @@ jobs:
- name: Verify downloadAllArtifacts()
shell: bash
run: |
scripts/test-artifact-file.sh "multi-artifact-directory/my-artifact-1/artifact-path/world.txt" "${{ env.non-gzip-artifact-content }}"
scripts/test-artifact-file.sh "multi-artifact-directory/my-artifact-2/artifact-path/gzip.txt" "${{ env.gzip-artifact-content }}"
packages/artifact/__tests__/test-artifact-file.sh "multi-artifact-directory/my-artifact-1/artifact-path/world.txt" "${{ env.non-gzip-artifact-content }}"
packages/artifact/__tests__/test-artifact-file.sh "multi-artifact-directory/my-artifact-2/artifact-path/gzip.txt" "${{ env.gzip-artifact-content }}"
8 changes: 4 additions & 4 deletions .github/workflows/cache-tests.yml
Expand Up @@ -47,11 +47,11 @@ jobs:

- name: Generate files in working directory
shell: bash
run: scripts/create-cache-files.sh ${{ runner.os }} test-cache
run: packages/cache/__tests__/create-cache-files.sh ${{ runner.os }} test-cache

- name: Generate files outside working directory
shell: bash
run: scripts/create-cache-files.sh ${{ runner.os }} ~/test-cache
run: packages/cache/__tests__/create-cache-files.sh ${{ runner.os }} ~/test-cache

# We're using node -e to call the functions directly available in the @actions/cache package
- name: Save cache using saveCache()
Expand All @@ -65,5 +65,5 @@ jobs:
- name: Verify cache
shell: bash
run: |
scripts/verify-cache-files.sh ${{ runner.os }} test-cache
scripts/verify-cache-files.sh ${{ runner.os }} ~/test-cache
packages/cache/__tests__/verify-cache-files.sh ${{ runner.os }} test-cache
packages/cache/__tests__/verify-cache-files.sh ${{ runner.os }} ~/test-cache
File renamed without changes.
12 changes: 6 additions & 6 deletions packages/cache/README.md
Expand Up @@ -2,6 +2,10 @@

> Functions necessary for caching dependencies and build outputs to improve workflow execution time.
See ["Caching dependencies to speed up workflows"](https://help.github.com/github/automating-your-workflow-with-github-actions/caching-dependencies-to-speed-up-workflows)for how caching works.

Note that GitHub will remove any cache entries that have not been accessed in over 7 days. There is no limit on the number of caches you can store, but the total size of all caches in a repository is limited to 5 GB. If you exceed this limit, GitHub will save your cache but will begin evicting caches until the total size is less than 5 GB.

## Usage

#### Restore Cache
Expand All @@ -24,7 +28,7 @@ const cacheKey = await cache.restoreCache(paths, key, restoreKeys)

#### Save Cache

Saves a cache containing the files in `paths` using the `key` provided. Function returns the cache id if the cache was save succesfully.
Saves a cache containing the files in `paths` using the `key` provided. The files would be compressed using zstandard compression algorithm if zstd is installed, otherwise gzip is used. Function returns the cache id if the cache was saved succesfully and throws an error if cache upload fails.

```js
const cache = require('@actions/cache');
Expand All @@ -34,8 +38,4 @@ const paths = [
]
const key = 'npm-foobar-d5ea0750'
const cacheId = await cache.saveCache(paths, key)
```

## Additional Documentation

See ["Caching dependencies to speed up workflows"](https://help.github.com/github/automating-your-workflow-with-github-actions/caching-dependencies-to-speed-up-workflows).
```
2 changes: 1 addition & 1 deletion packages/cache/RELEASES.md
@@ -1,5 +1,5 @@
# @actions/cache Releases

### 1.0.0
### 0.1.0

- Initial release
19 changes: 4 additions & 15 deletions packages/cache/__tests__/cacheUtils.test.ts
@@ -1,24 +1,11 @@
import * as io from '@actions/io'
import {promises as fs} from 'fs'
import * as path from 'path'
import * as cacheUtils from '../src/internal/cacheUtils'

jest.mock('@actions/core')
jest.mock('os')

function getTempDir(): string {
return path.join(__dirname, '_temp', 'cacheUtils')
}

afterAll(async () => {
delete process.env['GITHUB_WORKSPACE']
await io.rmRF(getTempDir())
})

test('getArchiveFileSize returns file size', () => {
test('getArchiveFileSizeIsBytes returns file size', () => {
const filePath = path.join(__dirname, '__fixtures__', 'helloWorld.txt')

const size = cacheUtils.getArchiveFileSize(filePath)
const size = cacheUtils.getArchiveFileSizeIsBytes(filePath)

expect(size).toBe(11)
})
Expand All @@ -28,6 +15,8 @@ test('unlinkFile unlinks file', async () => {
const testFile = path.join(testDirectory, 'test.txt')
await fs.writeFile(testFile, 'hello world')

await expect(fs.stat(testFile)).resolves.not.toThrow()

await cacheUtils.unlinkFile(testFile)

// This should throw as testFile should not exist
Expand Down
File renamed without changes.
44 changes: 16 additions & 28 deletions packages/cache/__tests__/restoreCache.test.ts
Expand Up @@ -12,6 +12,12 @@ jest.mock('../src/internal/cacheUtils')
jest.mock('../src/internal/tar')

beforeAll(() => {
jest.spyOn(console, 'log').mockImplementation(() => {})
jest.spyOn(core, 'debug').mockImplementation(() => {})
jest.spyOn(core, 'info').mockImplementation(() => {})
jest.spyOn(core, 'warning').mockImplementation(() => {})
jest.spyOn(core, 'error').mockImplementation(() => {})

// eslint-disable-next-line @typescript-eslint/promise-function-async
jest.spyOn(cacheUtils, 'getCacheFileName').mockImplementation(cm => {
const actualUtils = jest.requireActual('../src/internal/cacheUtils')
Expand Down Expand Up @@ -56,17 +62,13 @@ test('restore with no cache found', async () => {
const paths = ['node_modules']
const key = 'node-test'

const infoMock = jest.spyOn(core, 'info')
jest.spyOn(cacheHttpClient, 'getCacheEntry').mockImplementation(async () => {
return Promise.resolve(null)
})

const cacheKey = await restoreCache(paths, key)

expect(cacheKey).toBe(undefined)
expect(infoMock).toHaveBeenCalledWith(
`Cache not found for input keys: ${key}`
)
})

test('restore with server error should fail', async () => {
Expand All @@ -87,26 +89,19 @@ test('restore with restore keys and no cache found', async () => {
const key = 'node-test'
const restoreKey = 'node-'

const infoMock = jest.spyOn(core, 'info')

jest.spyOn(cacheHttpClient, 'getCacheEntry').mockImplementation(async () => {
return Promise.resolve(null)
})

const cacheKey = await restoreCache(paths, key, [restoreKey])

expect(cacheKey).toBe(undefined)
expect(infoMock).toHaveBeenCalledWith(
`Cache not found for input keys: ${key}, ${restoreKey}`
)
})

test('restore with gzip compressed cache found', async () => {
const paths = ['node_modules']
const key = 'node-test'

const infoMock = jest.spyOn(core, 'info')

const cacheEntry: ArtifactCacheEntry = {
cacheKey: key,
scope: 'refs/heads/master',
Expand All @@ -128,8 +123,8 @@ test('restore with gzip compressed cache found', async () => {
const downloadCacheMock = jest.spyOn(cacheHttpClient, 'downloadCache')

const fileSize = 142
const getArchiveFileSizeMock = jest
.spyOn(cacheUtils, 'getArchiveFileSize')
const getArchiveFileSizeIsBytesMock = jest
.spyOn(cacheUtils, 'getArchiveFileSizeIsBytes')
.mockReturnValue(fileSize)

const extractTarMock = jest.spyOn(tar, 'extractTar')
Expand All @@ -151,19 +146,18 @@ test('restore with gzip compressed cache found', async () => {
cacheEntry.archiveLocation,
archivePath
)
expect(getArchiveFileSizeMock).toHaveBeenCalledWith(archivePath)
expect(getArchiveFileSizeIsBytesMock).toHaveBeenCalledWith(archivePath)

expect(extractTarMock).toHaveBeenCalledTimes(1)
expect(extractTarMock).toHaveBeenCalledWith(archivePath, compression)

expect(unlinkFileMock).toHaveBeenCalledTimes(1)
expect(unlinkFileMock).toHaveBeenCalledWith(archivePath)

expect(infoMock).toHaveBeenCalledWith(`Cache restored from key: ${key}`)
expect(getCompressionMock).toHaveBeenCalledTimes(1)
})

test('restore with a pull request event and zstd compressed cache found', async () => {
test('restore with zstd compressed cache found', async () => {
const paths = ['node_modules']
const key = 'node-test'

Expand All @@ -189,8 +183,8 @@ test('restore with a pull request event and zstd compressed cache found', async
const downloadCacheMock = jest.spyOn(cacheHttpClient, 'downloadCache')

const fileSize = 62915000
const getArchiveFileSizeMock = jest
.spyOn(cacheUtils, 'getArchiveFileSize')
const getArchiveFileSizeIsBytesMock = jest
.spyOn(cacheUtils, 'getArchiveFileSizeIsBytes')
.mockReturnValue(fileSize)

const extractTarMock = jest.spyOn(tar, 'extractTar')
Expand All @@ -210,13 +204,11 @@ test('restore with a pull request event and zstd compressed cache found', async
cacheEntry.archiveLocation,
archivePath
)
expect(getArchiveFileSizeMock).toHaveBeenCalledWith(archivePath)
expect(getArchiveFileSizeIsBytesMock).toHaveBeenCalledWith(archivePath)
expect(infoMock).toHaveBeenCalledWith(`Cache Size: ~60 MB (62915000 B)`)

expect(extractTarMock).toHaveBeenCalledTimes(1)
expect(extractTarMock).toHaveBeenCalledWith(archivePath, compression)

expect(infoMock).toHaveBeenCalledWith(`Cache restored from key: ${key}`)
expect(getCompressionMock).toHaveBeenCalledTimes(1)
})

Expand Down Expand Up @@ -247,8 +239,8 @@ test('restore with cache found for restore key', async () => {
const downloadCacheMock = jest.spyOn(cacheHttpClient, 'downloadCache')

const fileSize = 142
const getArchiveFileSizeMock = jest
.spyOn(cacheUtils, 'getArchiveFileSize')
const getArchiveFileSizeIsBytesMock = jest
.spyOn(cacheUtils, 'getArchiveFileSizeIsBytes')
.mockReturnValue(fileSize)

const extractTarMock = jest.spyOn(tar, 'extractTar')
Expand All @@ -268,14 +260,10 @@ test('restore with cache found for restore key', async () => {
cacheEntry.archiveLocation,
archivePath
)
expect(getArchiveFileSizeMock).toHaveBeenCalledWith(archivePath)
expect(getArchiveFileSizeIsBytesMock).toHaveBeenCalledWith(archivePath)
expect(infoMock).toHaveBeenCalledWith(`Cache Size: ~0 MB (142 B)`)

expect(extractTarMock).toHaveBeenCalledTimes(1)
expect(extractTarMock).toHaveBeenCalledWith(archivePath, compression)

expect(infoMock).toHaveBeenCalledWith(
`Cache restored from key: ${restoreKey}`
)
expect(getCompressionMock).toHaveBeenCalledTimes(1)
})
12 changes: 10 additions & 2 deletions packages/cache/__tests__/saveCache.test.ts
@@ -1,16 +1,22 @@
import * as core from '@actions/core'
import * as path from 'path'
import {saveCache} from '../src/cache'
import * as cacheHttpClient from '../src/internal/cacheHttpClient'
import * as cacheUtils from '../src/internal/cacheUtils'
import {CacheFilename, CompressionMethod} from '../src/internal/constants'
import * as tar from '../src/internal/tar'

jest.mock('@actions/core')
jest.mock('../src/internal/cacheHttpClient')
jest.mock('../src/internal/cacheUtils')
jest.mock('../src/internal/tar')

beforeAll(() => {
jest.spyOn(console, 'log').mockImplementation(() => {})
jest.spyOn(core, 'debug').mockImplementation(() => {})
jest.spyOn(core, 'info').mockImplementation(() => {})
jest.spyOn(core, 'warning').mockImplementation(() => {})
jest.spyOn(core, 'error').mockImplementation(() => {})

// eslint-disable-next-line @typescript-eslint/promise-function-async
jest.spyOn(cacheUtils, 'getCacheFileName').mockImplementation(cm => {
const actualUtils = jest.requireActual('../src/internal/cacheUtils')
Expand Down Expand Up @@ -42,7 +48,9 @@ test('save with large cache outputs should fail', async () => {
const createTarMock = jest.spyOn(tar, 'createTar')

const cacheSize = 6 * 1024 * 1024 * 1024 //~6GB, over the 5GB limit
jest.spyOn(cacheUtils, 'getArchiveFileSize').mockReturnValueOnce(cacheSize)
jest
.spyOn(cacheUtils, 'getArchiveFileSizeIsBytes')
.mockReturnValueOnce(cacheSize)
const compression = CompressionMethod.Gzip
const getCompressionMock = jest
.spyOn(cacheUtils, 'getCompressionMethod')
Expand Down
File renamed without changes.
2 changes: 1 addition & 1 deletion packages/cache/package-lock.json

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

2 changes: 1 addition & 1 deletion packages/cache/package.json
@@ -1,6 +1,6 @@
{
"name": "@actions/cache",
"version": "1.0.0",
"version": "0.1.0",
"preview": true,
"description": "Actions cache lib",
"keywords": [
Expand Down
14 changes: 7 additions & 7 deletions packages/cache/src/cache.ts
Expand Up @@ -9,13 +9,15 @@ export class ValidationError extends Error {
constructor(message: string) {
super(message)
this.name = 'ValidationError'
Object.setPrototypeOf(this, ValidationError.prototype)
}
}

export class ReserveCacheError extends Error {
constructor(message: string) {
super(message)
this.name = 'ReserveCacheError'
Object.setPrototypeOf(this, ReserveCacheError.prototype)
}
}

Expand Down Expand Up @@ -47,7 +49,7 @@ function checkKey(key: string): void {
* @param paths a list of file paths to restore from the cache
* @param primaryKey an explicit key for restoring the cache
* @param restoreKeys an optional ordered list of keys to use for restoring the cache if no cache hit occurred for key
* @returns string returns the key for the cache hit, otherwise return undefined
* @returns string returns the key for the cache hit, otherwise returns undefined
*/
export async function restoreCache(
paths: string[],
Expand Down Expand Up @@ -78,7 +80,7 @@ export async function restoreCache(
compressionMethod
})
if (!cacheEntry?.archiveLocation) {
core.info(`Cache not found for input keys: ${keys.join(', ')}`)
// Cache not found
return undefined
}

Expand All @@ -92,7 +94,7 @@ export async function restoreCache(
// Download the cache from the cache entry
await cacheHttpClient.downloadCache(cacheEntry.archiveLocation, archivePath)

const archiveFileSize = utils.getArchiveFileSize(archivePath)
const archiveFileSize = utils.getArchiveFileSizeIsBytes(archivePath)
core.info(
`Cache Size: ~${Math.round(
archiveFileSize / (1024 * 1024)
Expand All @@ -109,8 +111,6 @@ export async function restoreCache(
}
}

core.info(`Cache restored from key: ${cacheEntry && cacheEntry.cacheKey}`)

return cacheEntry.cacheKey
}

Expand All @@ -120,7 +120,7 @@ export async function restoreCache(
* @param paths a list of file paths to be cached
* @param key an explicit key for restoring the cache
* @param options cache upload options
* @returns number returns cacheId if the cache was saved successfully
* @returns number returns cacheId if the cache was saved successfully and throws an error if save fails
*/
export async function saveCache(
paths: string[],
Expand Down Expand Up @@ -158,7 +158,7 @@ export async function saveCache(
await createTar(archiveFolder, cachePaths, compressionMethod)

const fileSizeLimit = 5 * 1024 * 1024 * 1024 // 5GB per repo limit
const archiveFileSize = utils.getArchiveFileSize(archivePath)
const archiveFileSize = utils.getArchiveFileSizeIsBytes(archivePath)
core.debug(`File Size: ${archiveFileSize}`)
if (archiveFileSize > fileSizeLimit) {
throw new Error(
Expand Down

0 comments on commit 4dcea16

Please sign in to comment.