Test/browser env #94
Test/browser env #94
Conversation
…. Add paused getter.
… browser-like env. Update deps in test to be compatible with env.
test/upchunk.spec.ts
Outdated
| // expect(isNumberArraySorted(progressPercentageArray)).toBeTruthy(); | ||
| // done(); | ||
| // }); | ||
| // }, 10000); |
There was a problem hiding this comment.
I'm not feeling good about losing this test.
tbh I think we should figure out how to keep it before this gets merged
There was a problem hiding this comment.
@dylanjha cool I think that's reasonable. I'll do some additional investigation. Hopefully it doesn't entail another replacement for nock.
|
An alternative to building xhr/xhr-mock each time is to check in vendored files for the tests and then have a script we can use to update them. xhr-mock was last updated in 2019 and xhr was last updated in 2020, so, incredibly unlikely they'll change. |
…R eval easier (DO NOT MERGE WITHOUT REVERTING).
test/web-test-runner.config.mjs
Outdated
| export default { | ||
| nodeResolve: true, | ||
| files: ['test/**/*.spec.js', 'test/**/*.spec.ts'], | ||
| // files: ['src/**/*.spec.js'], |
src/upchunk.spec.ts
Outdated
| @@ -1,333 +1,322 @@ | |||
| import * as nock from 'nock'; | |||
|
|
|||
| import { expect } from '@open-wc/testing'; | |||
There was a problem hiding this comment.
you're using expect to match what was here as closely as possible, right?
There was a problem hiding this comment.
yessir, jest uses bdd/expect by default (much more obvious in other test module diff).
src/upchunk.spec.ts
Outdated
| scopes.forEach((scope) => { | ||
| if (!scope.isDone()) { | ||
| done('All scopes not completed'); | ||
| /** @TODO Figure out how to best handle progress testing. See: https://www.npmjs.com/package/xhr-mock#upload-progress (CJP) */ |
There was a problem hiding this comment.
unfortunately, xhr-mock's progress events may not be fully-fleshed out enough for our needs here. jameslnewell/xhr-mock#82
An alternative is https://github.com/berniegp/mock-xmlhttprequest/#simulating-progress
…dded Content-Length header).
src/upchunk.spec.ts
Outdated
| const fileBytes = 1048576; | ||
| const upload = createUploadFixture( | ||
| { | ||
| headers: { |
There was a problem hiding this comment.
Here's the at least slightly dirty way of getting progress events working for xhr-mock in the same conditions as the previous test implementation using nock.
|
|
||
| const scopes = [ | ||
| nock('https://example.com') | ||
| .matchHeader('content-range', `bytes 0-${fileBytes / 4 - 1}/${fileBytes}`) |
There was a problem hiding this comment.
NOTE: I dropped these assertions/expectations from this particular test, since they weren't descriptive of what was being tested here and are already tested in a separate test specifically for these assertions/expectations (See test described as "a file is uploaded using the correct content-range headers")
This PR updates upchunk tests to run in a browser. To stay consistent with other projects (e.g. Open Elements, Media Chrome we've migrated to using web-test-runner. This also required some updates to tests and test dependencies that were either explicitly designed for node (
nockvs.xhr-mockas a replacement) and dependencies that implicitly assumed node without an appropriate build step (e.g.xhr, which only has a published package using cjs modules).A few callouts:
xhr-mock(mentioned in code comment for further details).pausedwas added to upchunk. This feels like a low risk refactor that is also plausibly useful for non-testing cases.xhr&xhr-mock) for compatibility in the new test env currently happens before each run of test. Since these dependencies should never change, they plausibly only need to be built once after a (local/dev) install. Since the build of these modules is very fast, solving this felt like a lower priority.