Skip to content

RetrieveCertificate: add mocked HTTP tests using real TPP responses#270

Merged
luispresuelVenafi merged 3 commits intoVenafi:masterfrom
maelvls:test-retrieve
Dec 1, 2022
Merged

RetrieveCertificate: add mocked HTTP tests using real TPP responses#270
luispresuelVenafi merged 3 commits intoVenafi:masterfrom
maelvls:test-retrieve

Conversation

@maelvls
Copy link
Copy Markdown
Contributor

@maelvls maelvls commented Nov 18, 2022

There are no tests that would allow us to "see" and discuss the issue reported in #171. Having tests, even statically mocked HTTP ones, would also allow us to prevent breakage (as much as possible) when changing RetrieveCertificate in #269.

I used curl and a 20.1 TPP VM to capture each individual HTTP response. At some point, I wish to document further how I produced these response samples.

The reason I went for a mocked HTTP server is because making the live TPP server return all of these messages is tricky; most 500 errors aren't easily reproducible unless we set up the e2e TPP VM to have e.g. one of its CA templates broken on purpose.

This "mocked HTTP server" approach isn't ideal because a drift may appear between earlier versions of TPP and these mocked responses, but I still think it is important to have them to prevent breakage when trying to refactor RetrieveCertificate.

@maelvls maelvls marked this pull request as draft November 29, 2022 11:14
@maelvls maelvls changed the title RetrieveCertificate: add mocked HTTP tests to showcase the unhelpful error messages RetrieveCertificate: add mocked HTTP tests using real TPP responses Nov 29, 2022
@maelvls maelvls marked this pull request as ready for review November 30, 2022 07:45
Copy link
Copy Markdown
Contributor

@inteon inteon left a comment

Choose a reason for hiding this comment

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

lgtm

We still use Go 1.17.

Signed-off-by: Maël Valais <mael@vls.dev>
Signed-off-by: Tim Ramlot <42113979+inteon@users.noreply.github.com>
@inteon
Copy link
Copy Markdown
Contributor

inteon commented Nov 30, 2022

@luispresuelVenafi This PR should be ready to merge now (all commits are signed).

@luispresuelVenafi luispresuelVenafi merged commit e607e30 into Venafi:master Dec 1, 2022
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.

4 participants