Skip to content

Run SwiftyRequest tests against local test server#61

Merged
djones6 merged 8 commits intomasterfrom
testServer
May 10, 2019
Merged

Run SwiftyRequest tests against local test server#61
djones6 merged 8 commits intomasterfrom
testServer

Conversation

@djones6
Copy link
Contributor

@djones6 djones6 commented May 3, 2019

Resolves #55

Also:

  • Includes a fix for self-signed certificates with templated URLs, where the RestRequest.url property was not being updated by the call to performSubstitutions(), and this was causing us to always take the default authentication handling route because URLComponents was trying (and failing) to initialize from a URL which still contained templated elements.
  • Enables logging output in the tests by copy-pasting PrintLogger (a simplified version of HeliumLogger) which we use in other repos' tests, eg. Kitura. Without this, you can't see error messages logged from SwiftyRequest as there is no logger registered, which makes debugging hard.
  • Resolved a local failure of the CircuitBreaker test, where the supposedly inaccessible URL was actually returning an HTTP response when connected to certain wifi networks. I've replaced it with a reference to localhost on a port that the server is not running on, which should be unaffected by other network connections.

@djones6 djones6 changed the title [WIP] Run SwiftyRequest tests against local test server Run SwiftyRequest tests against local test server May 9, 2019
@djones6 djones6 requested a review from Andrew-Lees11 May 9, 2019 14:59
}


// TODO: What does this test actually test?
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we want to keep this comment? This test is currently doing the same as testResponseData but with a Client certificate attached. It would be nice for our Kitura server to check the certificate in some way but i'm not sure if we even support 2 way SSL in Kitura using client certificates.

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 think we should keep it, because I think the test is misleading. We used to use self-signed.badssl.com with this test, which is not the right endpoint because it doesn't make use of client certificates (much as the current Kitura server does not).

However, badssl does actually provide a facility for testing client certificates: https://stackoverflow.com/questions/38095559/https-test-server-that-checks-client-certificates

We could try implementing this and see if we can make the test more meaningful.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OK - I've experimented with the client certificate test, and discovered that actually, the code that is supposedly providing client certificate support is completely broken. It was introduced in #34, but the test doesn't actually test client certificate support, and so I suppose it went unnoticed.

Fixing this is outside the scope of this PR - I'll raise a separate issue.

Copy link
Contributor

@Andrew-Lees11 Andrew-Lees11 left a comment

Choose a reason for hiding this comment

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

This is a nice solution to testing against a Kitura server without having to add Kitura as a dependency and removes our requirement on external APIs.

@djones6 djones6 merged commit 1e00600 into master May 10, 2019
@djones6 djones6 deleted the testServer branch May 10, 2019 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.

Remove external dependencies from tests

2 participants