Skip to content

Commit

Permalink
Fixes #357 Use a read stream for artifacts.
Browse files Browse the repository at this point in the history
This seems to be safe since octokit mentions ReadableStreams in their fetcher wrapper & tests: https://github.com/octokit/request.js/blob/main/src/fetch-wrapper.ts#L44
(and also it seems to work when I test it).
  • Loading branch information
ncipollo committed Aug 24, 2023
1 parent 2d19fb4 commit 6c75be8
Show file tree
Hide file tree
Showing 5 changed files with 31 additions and 29 deletions.
12 changes: 7 additions & 5 deletions __tests__/Artifact.test.ts
Original file line number Diff line number Diff line change
@@ -1,12 +1,14 @@
import { Artifact } from "../src/Artifact";
import {Artifact} from "../src/Artifact";

const fileContents = Buffer.from('artful facts', 'utf-8')
const contentLength = 42
const fakeReadStream = {}

jest.mock('fs', () => {
return {
readFileSync: () => fileContents,
statSync: () => { return { size: contentLength } }
createReadStream: () => fakeReadStream,
statSync: () => {
return {size: contentLength}
}
};
})

Expand All @@ -33,6 +35,6 @@ describe("Artifact", () => {

it('reads artifact', () => {
const artifact = new Artifact('some/artifact')
expect(artifact.readFile()).toBe(fileContents)
expect(artifact.readFile()).toBe(fakeReadStream)
})
})
38 changes: 19 additions & 19 deletions __tests__/ArtifactUploader.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ const artifacts = [
new Artifact('a/art1'),
new Artifact('b/art2')
]
const fileContents = Buffer.from('artful facts', 'utf-8')
const fakeReadStream = {}
const contentLength = 42
const releaseId = 100
const url = 'http://api.example.com'
Expand All @@ -19,7 +19,7 @@ const uploadMock = jest.fn()
jest.mock('fs', () => {
return {
promises: {},
readFileSync: () => fileContents,
createReadStream: () => fakeReadStream,
statSync: () => {
return {size: contentLength}
}
Expand All @@ -42,9 +42,9 @@ describe('ArtifactUploader', () => {

expect(uploadMock).toBeCalledTimes(2)
expect(uploadMock)
.toBeCalledWith(url, contentLength, 'raw', fileContents, 'art1', releaseId)
.toBeCalledWith(url, contentLength, 'raw', fakeReadStream, 'art1', releaseId)
expect(uploadMock)
.toBeCalledWith(url, contentLength, 'raw', fileContents, 'art2', releaseId)
.toBeCalledWith(url, contentLength, 'raw', fakeReadStream, 'art2', releaseId)

expect(deleteMock).toBeCalledTimes(0)
})
Expand All @@ -58,15 +58,15 @@ describe('ArtifactUploader', () => {

expect(uploadMock).toBeCalledTimes(5)
expect(uploadMock)
.toBeCalledWith(url, contentLength, 'raw', fileContents, 'art1', releaseId)
.toBeCalledWith(url, contentLength, 'raw', fakeReadStream, 'art1', releaseId)
expect(uploadMock)
.toBeCalledWith(url, contentLength, 'raw', fileContents, 'art1', releaseId)
.toBeCalledWith(url, contentLength, 'raw', fakeReadStream, 'art1', releaseId)
expect(uploadMock)
.toBeCalledWith(url, contentLength, 'raw', fileContents, 'art1', releaseId)
.toBeCalledWith(url, contentLength, 'raw', fakeReadStream, 'art1', releaseId)
expect(uploadMock)
.toBeCalledWith(url, contentLength, 'raw', fileContents, 'art2', releaseId)
.toBeCalledWith(url, contentLength, 'raw', fakeReadStream, 'art2', releaseId)
expect(uploadMock)
.toBeCalledWith(url, contentLength, 'raw', fileContents, 'art2', releaseId)
.toBeCalledWith(url, contentLength, 'raw', fakeReadStream, 'art2', releaseId)

expect(deleteMock).toBeCalledTimes(0)
})
Expand All @@ -81,9 +81,9 @@ describe('ArtifactUploader', () => {

expect(uploadMock).toBeCalledTimes(2)
expect(uploadMock)
.toBeCalledWith(url, contentLength, 'raw', fileContents, 'art1', releaseId)
.toBeCalledWith(url, contentLength, 'raw', fakeReadStream, 'art1', releaseId)
expect(uploadMock)
.toBeCalledWith(url, contentLength, 'raw', fileContents, 'art2', releaseId)
.toBeCalledWith(url, contentLength, 'raw', fakeReadStream, 'art2', releaseId)

expect(deleteMock).toBeCalledTimes(2)
expect(deleteMock).toBeCalledWith(1)
Expand All @@ -100,9 +100,9 @@ describe('ArtifactUploader', () => {

expect(uploadMock).toBeCalledTimes(2)
expect(uploadMock)
.toBeCalledWith(url, contentLength, 'raw', fileContents, 'art1', releaseId)
.toBeCalledWith(url, contentLength, 'raw', fakeReadStream, 'art1', releaseId)
expect(uploadMock)
.toBeCalledWith(url, contentLength, 'raw', fileContents, 'art2', releaseId)
.toBeCalledWith(url, contentLength, 'raw', fakeReadStream, 'art2', releaseId)

expect(deleteMock).toBeCalledTimes(0)
})
Expand All @@ -116,13 +116,13 @@ describe('ArtifactUploader', () => {

expect(uploadMock).toBeCalledTimes(4)
expect(uploadMock)
.toBeCalledWith(url, contentLength, 'raw', fileContents, 'art1', releaseId)
.toBeCalledWith(url, contentLength, 'raw', fakeReadStream, 'art1', releaseId)
expect(uploadMock)
.toBeCalledWith(url, contentLength, 'raw', fileContents, 'art1', releaseId)
.toBeCalledWith(url, contentLength, 'raw', fakeReadStream, 'art1', releaseId)
expect(uploadMock)
.toBeCalledWith(url, contentLength, 'raw', fileContents, 'art1', releaseId)
.toBeCalledWith(url, contentLength, 'raw', fakeReadStream, 'art1', releaseId)
expect(uploadMock)
.toBeCalledWith(url, contentLength, 'raw', fileContents, 'art2', releaseId)
.toBeCalledWith(url, contentLength, 'raw', fakeReadStream, 'art2', releaseId)

expect(deleteMock).toBeCalledTimes(0)
})
Expand Down Expand Up @@ -164,9 +164,9 @@ describe('ArtifactUploader', () => {

expect(uploadMock).toBeCalledTimes(2)
expect(uploadMock)
.toBeCalledWith(url, contentLength, 'raw', fileContents, 'art1', releaseId)
.toBeCalledWith(url, contentLength, 'raw', fakeReadStream, 'art1', releaseId)
expect(uploadMock)
.toBeCalledWith(url, contentLength, 'raw', fileContents, 'art2', releaseId)
.toBeCalledWith(url, contentLength, 'raw', fakeReadStream, 'art2', releaseId)

expect(deleteMock).toBeCalledTimes(0)
})
Expand Down
2 changes: 1 addition & 1 deletion dist/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -205,7 +205,7 @@ class Artifact {
return (0, fs_1.statSync)(this.path).size;
}
readFile() {
return (0, fs_1.readFileSync)(this.path);
return (0, fs_1.createReadStream)(this.path);
}
}
exports.Artifact = Artifact;
Expand Down
2 changes: 1 addition & 1 deletion dist/index.js.map

Large diffs are not rendered by default.

6 changes: 3 additions & 3 deletions src/Artifact.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { basename } from "path";
import { readFileSync, statSync } from "fs";
import {createReadStream, readFileSync, ReadStream, statSync} from "fs";

export class Artifact {
readonly contentType: string
Expand All @@ -16,7 +16,7 @@ export class Artifact {
return statSync(this.path).size
}

readFile(): Buffer {
return readFileSync(this.path)
readFile(): ReadStream {
return createReadStream(this.path)
}
}

0 comments on commit 6c75be8

Please sign in to comment.