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.

Sign up for GitHub

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

Jump to bottom

@uppy/companion: fix infinite recursion in uploader test #4536

Merged
merged 2 commits into from
Jul 6, 2023
Merged

@uppy/companion: fix infinite recursion in uploader test #4536

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
66e467d
fix stack size exceeded
mifi Jun 28, 2023
2641763
remove invalid assertion
mifi Jun 28, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions packages/@uppy/companion/test/__tests__/uploader.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -118,14 +118,15 @@ describe('uploader with tus protocol', () => {
}

const uploader = new Uploader(opts)
const originalTryDeleteTmpPath = uploader.tryDeleteTmpPath.bind(uploader)
uploader.tryDeleteTmpPath = async () => {
// validate that the tmp file has been downloaded and saved into the file path
// must do it before it gets deleted
const fileInfo = fs.statSync(uploader.tmpPath)
expect(fileInfo.isFile()).toBe(true)
expect(fileInfo.size).toBe(fileContent.length)

return uploader.tryDeleteTmpPath()
return originalTryDeleteTmpPath()
}
const uploadToken = uploader.token
expect(uploadToken).toBeTruthy()
Expand All @@ -148,13 +149,12 @@ describe('uploader with tus protocol', () => {
// emulate socket connection
socketClient.connect(uploadToken)
socketClient.onProgress(uploadToken, (message) => {
if (firstReceivedProgress == null) firstReceivedProgress = message.payload.bytesUploaded
if (firstReceivedProgress == null) firstReceivedProgress = message.payload
})
socketClient.onUploadSuccess(uploadToken, (message) => {
try {
expect(message.payload.bytesTotal).toBe(fileContent.length)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why are we no longer expecting that?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

in this commit 1b5c0d7#diff-04b10f44d32399240120a15fe7a11a634d53760af469bf3ef80411421a164055L149 I accidentally moved that expect from onProgress. Turns out that onSuccess doesn't have any payload.bytesTotal, but we never knew because this test succeeded with a false positive due to a different problem (that this PR also fixes)

expect(firstReceivedProgress.bytesUploaded).toBe(8192)

expect(firstReceivedProgress).toBe(8192)
// see __mocks__/tus-js-client.js
expect(message.payload.url).toBe('https://tus.endpoint/files/foo-bar')
} catch (err) {
Expand Down