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.

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

Remove tests we don't trust #124

Merged
merged 1 commit into from
Nov 26, 2019
Merged

Remove tests we don't trust #124

merged 1 commit into from
Nov 26, 2019

Conversation

srallen
Copy link
Contributor

@srallen srallen commented Nov 25, 2019

The auth tests weren't trustworthy for #121, so this removes them, because bad tests are worse than no tests.

@srallen srallen requested a review from camallen November 25, 2019 19:21
Copy link
Contributor

@camallen camallen left a comment

Choose a reason for hiding this comment

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

It would be good to know why these tests fail, i kept getting a failure when parsing the json response object after registering (API side was a 201 response).

I'm ok with removing these in favour of integration testing, though it would have been ideal to replace these with decent unit tests.

Is there any way we can automate the integration tests using something like https://github.com/puppeteer/puppeteer ?

@srallen
Copy link
Contributor Author

srallen commented Nov 26, 2019

Yeah puppeteer should work. I'd also use nock. We'll need to circle back to this soon.

@srallen srallen merged commit 2bdc6e4 into master Nov 26, 2019
@srallen srallen deleted the remove-tests branch November 26, 2019 15:41
@wgranger wgranger mentioned this pull request Jul 16, 2020
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