Skip to content

Conversation

@brizental
Copy link
Contributor

Adding tests for the timeout mechanism and cookie removal is blocked because we need to figure out the mest way bot to mock fetch on these tests.

Filed a bug to do that later, for now I manually tested and it "works on my machine"™.

@brizental brizental requested a review from Dexterp37 January 18, 2021 16:27
* License, v. 2.0. If a copy of the MPL was not distributed with this
* file, You can obtain one at http://mozilla.org/MPL/2.0/. */

import "jsdom-global/register";
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I literally only have this because it includes DOMException. One less thing for me to mock.


const sandbox = sinon.createSandbox();

global.fetch = () => {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I know this code is hacky AF. But the bug I opened to look into mocking fetch is what should fix it.

export interface UploadAdapter {
export abstract class Uploader {
// The timeout, in seconds, to use for all operations with the server.
protected defaultTimeout = 10_000;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I copied this from the python bindings, on the android bindings we have two different timeouts: one for connection timeout and another for request timeout. I don't know that there is a way to make this differentiation here.

Copy link
Contributor

Choose a reason for hiding this comment

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

It's fine to not have the differentiation if the uploader doesn't allow it :)

Copy link
Contributor

@Dexterp37 Dexterp37 left a comment

Choose a reason for hiding this comment

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

r+wc

export interface UploadAdapter {
export abstract class Uploader {
// The timeout, in seconds, to use for all operations with the server.
protected defaultTimeout = 10_000;
Copy link
Contributor

Choose a reason for hiding this comment

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

It's fine to not have the differentiation if the uploader doesn't allow it :)

@brizental brizental merged commit f587679 into mozilla:main Jan 19, 2021
@brizental brizental deleted the 1677440-uploader branch January 19, 2021 17:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants