Skip to content

Commit

Permalink
Replay progress events in unit tests (#1017)
Browse files Browse the repository at this point in the history
Skips the browser, http, network etc. and lets us test the smallest unit of our internals – an UploadFile having its state updated by a series of ProgressEvent events.

For each scenario I'm pretty sure we only need to test a single file with a stream of synchronous events.

Covers some recorded events from different browsers and endpoints. We can add more scenarios here if we come across them.

Specifically, the Chrome scenario proves the fix in #1016 – this was previously failing early with 100% progress, which is the same issue reported by users in #1013

I've painstakingly copied values from log screenshots to setup this existing suite. Hopefully contributors can PR full tests in future to make things a bit easier 😅

Fixes #1013
  • Loading branch information
gilest authored Nov 10, 2023
1 parent ccf2363 commit afd70da
Show file tree
Hide file tree
Showing 3 changed files with 326 additions and 40 deletions.
4 changes: 4 additions & 0 deletions ember-file-upload/src/internal.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,14 @@
import DataTransferWrapper from './system/data-transfer-wrapper.ts';
import HTTPRequest from './system/http-request.ts';
import UploadFileReader from './system/upload-file-reader.ts';
import { onloadstart, onprogress, onloadend } from './system/upload.ts';

export {
// Non-public classes imported by the test app
DataTransferWrapper,
HTTPRequest,
UploadFileReader,
onloadstart,
onprogress,
onloadend,
};
93 changes: 53 additions & 40 deletions ember-file-upload/src/system/upload.ts
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,56 @@ function normalizeOptions(
return options;
}

export function onloadstart(
file: UploadFile,
event?: ProgressEvent<EventTarget>,
) {
if (!event) return;
if (!event.lengthComputable || event.total === 0) return;

file.loaded = event.loaded;
// It occurs that the event.total is not always correct.
// For this reason we should hold the max file size.
// The correct should be returned while progress
file.size = Math.max(file.size, event.total);
file.progress = (file.loaded / file.size) * 100;
}

export function onprogress(
file: UploadFile,
event?: ProgressEvent<EventTarget>,
) {
if (!event) return;
// We need to check also for isUploadComplete, because the browsers brings sometimes the onprogress after onloadend event
if (!event.lengthComputable || event.total === 0 || file.isUploadComplete)
return;

file.size = event.total;

// When the progress is completed there is possible that we get the `Content-Length` response header of the upload endpoint as loaded / total.
// There is possible that `Content-Length` is lower or higher than the already loaded bytes.
// if there is lower, we want to keep the higher loaded value, otherwise the progress percentage will be decreased
// When the event.loaded is higher than the start file.size, we use the file.size, otherwise it can occur that progress for the file is higher than 100%
let loaded = event.loaded;
if (loaded > file.size) {
loaded = file.size;
}
file.loaded = Math.max(loaded, file.loaded);
file.progress = (file.loaded / file.size) * 100;
}

export function onloadend(
file: UploadFile,
event?: ProgressEvent<EventTarget>,
) {
if (!event) return;
if (!event.lengthComputable || event.total === 0) return;

file.loaded = file.size;
file.progress = (file.loaded / file.size) * 100;
file.isUploadComplete = true;
}

export function upload(
file: UploadFile,
url: string | object,
Expand Down Expand Up @@ -87,46 +137,9 @@ export function upload(
request.timeout = options.timeout;
}

request.onloadstart = function (evt) {
if (!evt) return;
if (!evt.lengthComputable || evt.total === 0) return;

file.loaded = evt.loaded;
// It occurs that the evt.total is not always correct.
// For this reason we should hold the max file size.
// The correct should be returned while progress
file.size = Math.max(file.size, evt.total);
file.progress = (file.loaded / file.size) * 100;
};

request.onprogress = function (evt) {
if (!evt) return;
// We need to check also for isUploadComplete, because the browsers brings sometimes the onprogress after onloadend event
if (!evt.lengthComputable || evt.total === 0 || file.isUploadComplete)
return;

file.size = evt.total;

// When the progress is completed there is possible that we get the `Content-Length` response header of the upload endpoint as loaded / total.
// There is possible that `Content-Length` is lower or higher than the already loaded bytes.
// if there is lower, we want to keep the higher loaded value, otherwise the progress percentage will be decreased
// When the evt.loaded is higher than the start file.size, we use the file.size, otherwise it can occur that progress for the file is higher than 100%
let loaded = evt.loaded;
if (loaded > file.size) {
loaded = file.size;
}
file.loaded = Math.max(loaded, file.loaded);
file.progress = (file.loaded / file.size) * 100;
};

request.onloadend = function (evt) {
if (!evt) return;
if (!evt.lengthComputable || evt.total === 0) return;

file.loaded = file.size;
file.progress = (file.loaded / file.size) * 100;
file.isUploadComplete = true;
};
request.onloadstart = (event) => onloadstart(file, event);
request.onprogress = (event) => onprogress(file, event);
request.onloadend = (event) => onloadend(file, event);

request.ontimeout = () => {
file.state = FileState.TimedOut;
Expand Down
269 changes: 269 additions & 0 deletions test-app/tests/unit/system/upload-progress-test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,269 @@
/* eslint-disable qunit/no-conditional-assertions */

import { module, test } from 'qunit';
import { setupTest } from 'ember-qunit';
import { FileSource, UploadFile } from 'ember-file-upload';
import { type TestContext } from '@ember/test-helpers';
import { png } from 'test-app/tests/utils/file-data';
import { onloadstart, onprogress, onloadend } from 'ember-file-upload/internal';

interface Handlers {
[key: string]: typeof onloadstart;
}

const handlers: Handlers = {
loadstart: onloadstart,
progress: onprogress,
loadend: onloadend,
};

type Step = {
assertProgressBefore?: number;
event: ProgressEvent;
assertProgressAfter?: number;
};

const replayEvents = (
assert: Assert,
uploadFile: UploadFile,
steps: Step[],
) => {
for (const step of steps) {
if (typeof step.assertProgressBefore === 'number') {
assert.strictEqual(
uploadFile.progress,
step.assertProgressBefore,
`before (index: ${steps.indexOf(step)}), progress is: ${
step.assertProgressBefore
}% type: ${step.event.type}, total: ${step.event.total}, loaded: ${
step.event.loaded
}`,
);
}

const handler = handlers[step.event.type];
if (!handler) throw new Error(`No handler matched ${step.event.type}`);

handler(uploadFile, step.event);

if (typeof step.assertProgressAfter === 'number') {
assert.strictEqual(
uploadFile.progress,
step.assertProgressAfter,
`after (index: ${steps.indexOf(step)}), progress is: ${
step.assertProgressAfter
}% type: ${step.event.type}, total: ${step.event.total}, loaded: ${
step.event.loaded
}`,
);
}
}
};

module('Unit | upload progress', function (hooks) {
setupTest(hooks);

test('nominal progress (recorded from Firefox in issue #1002)', function (this: TestContext, assert) {
const uploadFile = new UploadFile(
new File(png, 'test-filename.png'),
FileSource.Browse,
);

const steps = [
{
assertProgressBefore: 0,
event: new ProgressEvent('loadstart', {
lengthComputable: true,
loaded: 0,
total: 0,
}),
},
{
event: new ProgressEvent('loadstart', {
lengthComputable: true,
loaded: 0,
total: 618220,
}),
assertProgressAfter: 0,
},
{
assertProgressBefore: 0,
event: new ProgressEvent('progress', {
lengthComputable: true,
loaded: 30954,
total: 618220,
}),
assertProgressAfter: 5.0069554527514475,
},
{
event: new ProgressEvent('progress', {
lengthComputable: true,
loaded: 62954,
total: 618220,
}),
assertProgressAfter: 10.18310633754974,
},
];

replayEvents(assert, uploadFile, steps);

assert.strictEqual(
uploadFile.size,
618220,
'upload filesize is set from handlers',
);
});

test('regression: content length of final `progress` and `loadend` is less than file size (Issue #1002)', function (this: TestContext, assert) {
const uploadFile = new UploadFile(
new File(png, 'test-filename.png'),
FileSource.Browse,
);

const steps = [
{
assertProgressBefore: 0,
event: new ProgressEvent('progress', {
lengthComputable: true,
loaded: 248723,
total: 248723,
}),
assertProgressAfter: 100,
},
{
assertProgressBefore: 100,
event: new ProgressEvent('loadend', {
lengthComputable: true,
loaded: 248723,
total: 248723,
}),
assertProgressAfter: 100,
},
{
assertProgressBefore: 100,
event: new ProgressEvent('progress', {
lengthComputable: true,
loaded: 390,
total: 390,
}),
assertProgressAfter: 100,
},
{
assertProgressBefore: 100,
event: new ProgressEvent('loadend', {
lengthComputable: true,
loaded: 390,
total: 390,
}),
assertProgressAfter: 100,
},
];

replayEvents(assert, uploadFile, steps);

assert.strictEqual(
uploadFile.size,
248723,
'upload filesize is set from handlers',
);
});

test('regression: incorrect `total` in second `loadstart` event (recorded from Google Chrome in issue #1002)', function (this: TestContext, assert) {
const uploadFile = new UploadFile(
new File(png, 'test-filename.png'),
FileSource.Browse,
);

const steps = [
{
assertProgressBefore: 0,
event: new ProgressEvent('loadstart', {
lengthComputable: true,
loaded: 0,
total: 0,
}),
},
{
event: new ProgressEvent('loadstart', {
lengthComputable: true,
loaded: 0,
total: 712,
}),
},
{
assertProgressBefore: 0,
event: new ProgressEvent('progress', {
lengthComputable: true,
loaded: 16384,
total: 618122,
}),
assertProgressAfter: 2.6506094266180464,
},
{
event: new ProgressEvent('progress', {
lengthComputable: true,
loaded: 32768,
total: 618122,
}),
assertProgressAfter: 5.301218853236093,
},
];

replayEvents(assert, uploadFile, steps);

assert.strictEqual(
uploadFile.size,
618122,
'upload filesize is set from handlers',
);
});

test('final response has `Content-Length: 0` (loadend with total: 0)', function (this: TestContext, assert) {
const uploadFile = new UploadFile(
new File(png, 'test-filename.png'),
FileSource.Browse,
);

const steps = [
{
assertProgressBefore: 0,
event: new ProgressEvent('loadstart', {
lengthComputable: true,
loaded: 0,
total: 0,
}),
},
{
event: new ProgressEvent('loadstart', {
lengthComputable: true,
loaded: 0,
total: 618220,
}),
},
{
event: new ProgressEvent('progress', {
lengthComputable: true,
loaded: 618220,
total: 618220,
}),
assertProgressAfter: 100,
},
{
event: new ProgressEvent('loadend', {
lengthComputable: true,
loaded: 0,
total: 0,
}),
assertProgressAfter: 100,
},
];

replayEvents(assert, uploadFile, steps);

assert.strictEqual(
uploadFile.size,
618220,
'upload filesize is set from handlers',
);
});
});

0 comments on commit afd70da

Please sign in to comment.