-
Notifications
You must be signed in to change notification settings - Fork 122
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
docs: Add guidelines for e2e tests #1466
Conversation
d1ab5e3
to
154d56d
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great work @p-offtermatt. See my comments below.
Co-authored-by: Marius Poke <marius.poke@posteo.de>
Co-authored-by: Marius Poke <marius.poke@posteo.de>
Co-authored-by: Marius Poke <marius.poke@posteo.de>
…n-security into ph/e2e-tutorial
Co-authored-by: Marius Poke <marius.poke@posteo.de>
Co-authored-by: Marius Poke <marius.poke@posteo.de>
…n-security into ph/e2e-tutorial
Co-authored-by: Marius Poke <marius.poke@posteo.de>
Co-authored-by: Marius Poke <marius.poke@posteo.de>
Co-authored-by: Marius Poke <marius.poke@posteo.de>
…n-security into ph/e2e-tutorial
@@ -1,4 +1,188 @@ | |||
## Updating the trace format and tests when adjusting the framework | |||
# End-to-end testing | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would be nice to have a tl;dr or cheat sheet here, about running different kinds of tests & traces with different config.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you think go run ./tests/e2e/... --help
explains it properly?
I added a note to run that. If you think it's not helpful and there should be more here,
please lmk, otherwise just resolve this :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bar the tl;dr additional, good overall.
// potentially check something in the output, or log something, ... | ||
``` | ||
|
||
Don't forget to wire your action into [main.go](main.go):runStep, where |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: Why not point direclty to the line inside main.go
that containsrunStep
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I dislike it because it will not point to the right place anymore extremely quickly, i.e. as soon as the file is touched and this is not adjusted.
I'd be happy about suggestions on how to overcome this! I know anchors exist, but I think they make the source file a bit verbose.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Great doc!
Co-authored-by: Simon Noetzlin <simon.ntz@gmail.com>
Thanks everyone for the reviews! 🙌 I think everyone had a chance to review, so I will merge this :) |
Description
Closes: #1334
This adds guidelines on how to write e2e tests to tests/e2e/README.md
TODO: add this README to the .md link checker, potentially?
Author Checklist
All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow up issues.
I have...
docs:
prefix in the PR titleReviewers Checklist
All items are required. Please add a note if the item is not applicable and please add
your handle next to the items reviewed if you only reviewed selected items.
I have...
docs:
prefix in the PR titlemake build-docs
)