Skip to content
This repository has been archived by the owner on May 2, 2022. It is now read-only.

Prevent case that login search string is empty #9

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

linonetwo
Copy link

Might prevent a situation like #8

Copy link
Contributor

@jaxoncreed jaxoncreed left a comment

Choose a reason for hiding this comment

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

Could you provide a unit test for this?

Copy link
Contributor

@jaxoncreed jaxoncreed left a comment

Choose a reason for hiding this comment

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

Perhaps it would be better to error rather than creating an empty object.

@linonetwo
Copy link
Author

Ok, I simply throw an error instead.

Copy link
Contributor

@jaxoncreed jaxoncreed left a comment

Choose a reason for hiding this comment

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

I would still be great to have a simple unit test for this.

src/SolidClient.js Outdated Show resolved Hide resolved
src/SolidClient.js Outdated Show resolved Hide resolved
src/SolidClient.js Outdated Show resolved Hide resolved
@linonetwo
Copy link
Author

linonetwo commented Jan 11, 2020

I'm going to add a test case, using jest.

@linonetwo
Copy link
Author

const request = https.request(options);

isn't easy to mock using jest-fetch mock, would you mind I refactor it to node-fetch?

@jaxoncreed
Copy link
Contributor

I think that should be fine @linonetwo

Base automatically changed from master to main February 24, 2021 18:41
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants