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

Add more HTTPS tests #1430

Merged
merged 12 commits into from Sep 2, 2020
Merged

Add more HTTPS tests #1430

merged 12 commits into from Sep 2, 2020

Conversation

Giotino
Copy link
Collaborator

@Giotino Giotino commented Aug 31, 2020

Some tests for the HTTPS options.

These tests simulate a real PKI application.
E.g. https://github.com/Giotino/got/blob/ca8d81314ac93bacd5e0c1da207210adf4e85a4a/test/https.ts#L232-L248

Types are messed up, I'm going to fix them soon, I'm also going to add support for pfx, needed for #1364.

This is a draft, I'm going to add more tests covering more functionalities (e.g. key passphrase or invalid certificates).

create-cert-test-server.ts is like https://github.com/lukechilds/create-test-server, but more specific for our needs.

TODO List

  • Add PFX test environment
  • Fix types

Checklist

  • I have read the documentation.
  • I have included a pull request description of my changes.
  • I have included some tests.
  • If it's a new feature, I have included documentation updates in both the README and the types.

@Giotino
Copy link
Collaborator Author

Giotino commented Aug 31, 2020

In order to do more deeper HTTPS tests it has been needed to replace https://github.com/lukechilds/create-test-server for those tests.

The new create-https-test-server is an helper in the got test suite. It's extensible and can easily be adapted to create an HTTPS server with the need of every single tests, this is going to be useful when new HTTPS options are going to be added (e.g. the pfx PR).

This has also enabled me to remove external dependencies for testing the expired certificate and the wrong host (https://github.com/sindresorhus/got/pull/1430/files#diff-ef513147d9bbe98f3dc4e9dc04783109L123-R132).

All that said I think this PR is ready for some review.
@szmarczak @sindresorhus

@Giotino Giotino marked this pull request as ready for review August 31, 2020 20:09
@szmarczak
Copy link
Collaborator

Does the tests pass on Windows?

@Giotino
Copy link
Collaborator Author

Giotino commented Aug 31, 2020

Does the tests pass on Windows?

Yes, I'm running it on Windows

@sindresorhus sindresorhus changed the title HTTPS Tests Add more HTTPS tests Sep 1, 2020
@szmarczak szmarczak merged commit 4ff7098 into sindresorhus:master Sep 2, 2020
@Giotino Giotino deleted the https-tests branch September 2, 2020 13:16
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.

None yet

3 participants