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

Webhook test coverage #15537

Closed
28 of 29 tasks
naz opened this issue Oct 6, 2022 · 26 comments
Closed
28 of 29 tasks

Webhook test coverage #15537

naz opened this issue Oct 6, 2022 · 26 comments
Labels
good first issue [triage] Start here if you've never contributed before. Hacktoberfest Issues suitable for hacktoberfest participants help wanted [triage] Ideal issues for contributors to help with

Comments

@naz
Copy link
Contributor

naz commented Oct 6, 2022

Webhooks have been a part of Ghost since very first versions. This area has been constantly lagging behind with solid test coverage. Having tests would add confidence to webhooks stability. Snapshot tests would be a great source of example data webhooks deliver to external integrations.

To improve this situation, Ghost has introduced a special purpose Webhooks snapshot tests. These are snapshot-based tests (similar to the end-to-end API tests) with one big difference - they test the outgoing request payload instead of API responses.

To understand how end-to-end testing works at Ghost have a read through this guide. The concepts applied in end-to-end API tests are very similar to ones used in webhook tests.

Here is an example of a webhook test suite for for post.published and post.added events.

The above suite covers two event's which Webhook events listen to. There are 29 total events Webhooks can respond to, 27 left to cover. We need help writing tests for them!

Below are some pointers around what you could help us with. For the most part it's covering all of the webhook events with at least one "happy path" test.

Steps to cover a webhook event listener with a test

1. Write a test

First, find one webhook event listener to cover - search for an event that has no test coverage in the list of all webhook event listeners.

Copy of all events below for quick reference and ✅ for the ones with at least one test:

Before digging deep into work please double check if the list is up to date - check if there are any open or closed Pull Requests open with the name of the event you are about to pick up.

Let's pick site.changed event to cover with example a test.

Webhook tests are located in ghost/core/test/e2e-webhooks/{model_name}.test.js (e.g.: test/e2e-webhooks/posts.test.js). As there is no site.test.js suite in the e2e-webhooks folder - create a site.test.js file. Note, for post.rescheduled event, there is a posts.test.js test suite already. In this case, we would add test cases to existing posts.test.js test suite. The convention is to group test suites by common model name - {model_name}.test.js.

Write the test in the newly created file following concepts in end-to-end testing guide. For an easy start it's usually a good idea to copy a similar working test suite into your new file and tweak it.

Notes on Webhook test case structure

Webhook tests follow a setup->action->assert structure used commonly when writing other types of tests, like unit tests:

  1. Setup
  1. Action
  • Find under which conditions the event under test is triggered (usually a result of API call(s)). This might take a little bit of research and experimentation. If you are completely lost and did not find the pathway after searching feel free to leave a comment here for more guidance.
  1. Assertion

2. Run the test and record a snapshot

Use the yarn test:single command inside ghost/core to run the test suite you've been working on:

yarn test:single test/e2e-webhooks/site.test.js

After the test run, there will be a snapshot file created with recorder webhook payload (example).

Check the Snapshot Testing section of the end-to-end testing guide for more information about the inner workings of snapshot-based tests.

3. Open a PR

To make sure you've not broken anything & followed our code standard, run yarn test:all. You can also use yarn lint to check formatting issues.

Commit and push the test suite code and the snapshot to your working branch. Open up a separate PR for each webhook even you cover.

🎉 All done! we will try to review your contribution ASAP.


Please follow our contribution guidelines as closely as you can, particularly being sure to reference this issue in your commit. Test coverage commits should not use emojis as they are not user-facing changes.

This issue is going to improve the ease of bug reproductions and will let all Ghost developers sleep better after modifying webhook-related code. This means a lot for the team and contributors!

Thank you 🙏 💚

@naz naz added good first issue [triage] Start here if you've never contributed before. help wanted [triage] Ideal issues for contributors to help with Hacktoberfest Issues suitable for hacktoberfest participants labels Oct 6, 2022
@github-actions github-actions bot added needs:triage [triage] this needs to be triaged by the Ghost team and removed needs:triage [triage] this needs to be triaged by the Ghost team labels Oct 6, 2022
@dshubhadeep
Copy link
Contributor

hi @naz
I would like to work on tag.* events

@naz
Copy link
Contributor Author

naz commented Oct 6, 2022

Hey @dshubhadeep 👋 Feel free to pick one event at a time, will try my best to review ASAP. And feel free to ask questions if anything is unclear!

@dshubhadeep
Copy link
Contributor

Thanks, I'll start with tag.added event.

@illiteratewriter
Copy link
Contributor

I'd like to work on member.added event

@JohnGrisham
Copy link
Contributor

I'll try my hand at page.added

naz added a commit that referenced this issue Oct 7, 2022
refs #15537
refs 4110ffa

- The test had minor formatting issues not worth an extra back-forth during the PR review
illiteratewriter added a commit to illiteratewriter/Ghost that referenced this issue Oct 7, 2022
@jennyxchang
Copy link
Contributor

I can work on post.scheduled event.

daniellockyer pushed a commit that referenced this issue Oct 7, 2022
refs #15537

- this adds an e2e test and test snapshot for the `tag.deleted` webhook so we can prevent regressions and bugs in the future
daniellockyer pushed a commit that referenced this issue Oct 7, 2022
refs #15537

- this adds an e2e test and test snapshot for the `member.added` webhook so we can prevent regressions and bugs in the future
dshubhadeep added a commit to dshubhadeep/Ghost that referenced this issue Oct 7, 2022
refs TryGhost#15537
- this adds an e2e test and test snapshot for the `tag.edited` webhook so we can prevent regressions and bugs in the future
@ErisDS ErisDS changed the title Test coverage for Ghost's Webhooks Webhook test coverage Oct 7, 2022
illiteratewriter added a commit to illiteratewriter/Ghost that referenced this issue Oct 8, 2022
@kritikash18
Copy link
Contributor

working on post.deleted event

ErisDS pushed a commit to Dark-Knight11/Ghost that referenced this issue Oct 30, 2022
ref TryGhost#15537

- this adds an e2e test and test snapshot for the page.published webhook
so we can prevent regressions and bugs in the future
ErisDS pushed a commit to Dark-Knight11/Ghost that referenced this issue Oct 30, 2022
refs: TryGhost#15537

This adds an e2e test and test snapshot for the page.tag.detached webhook
so we can prevent regressions and bugs in the future
ErisDS pushed a commit that referenced this issue Oct 30, 2022
refs: #15537

- snapshot test created to add confidence to webhook stability and increase overall test coverage.
ErisDS pushed a commit that referenced this issue Oct 30, 2022
refs: #15537

- snapshot test created to add confidence to webhook stability and increase overall test coverage.
ErisDS pushed a commit to RobinCsl/Ghost that referenced this issue Oct 31, 2022
ErisDS pushed a commit that referenced this issue Oct 31, 2022
refs: #15537

- snapshot test created to add confidence to webhook stability and increase overall test coverage.
ErisDS pushed a commit to RobinCsl/Ghost that referenced this issue Oct 31, 2022
ErisDS pushed a commit that referenced this issue Nov 2, 2022
refs: #15537

- snapshot test created to add confidence to webhook stability and increase overall test coverage.
@ErisDS
Copy link
Member

ErisDS commented Nov 2, 2022

We now have either a merged or open PR for every single webhook. Thank you to everyone who contributed ❤️ this is incredible - you've all helped us to make a sea-change in our test coverage and ability to support webhooks and zapier going forward.

Thank you 🙏

@ErisDS ErisDS closed this as completed Nov 2, 2022
ErisDS pushed a commit that referenced this issue Nov 5, 2022
refs: #15537

- snapshot test created to add confidence to webhook stability and increase overall test coverage.
ErisDS pushed a commit that referenced this issue Nov 5, 2022
refs: #15537

- snapshot test created to add confidence to webhook stability and increase overall test coverage.
RobinCsl added a commit to RobinCsl/Ghost that referenced this issue Jan 31, 2023
ErisDS pushed a commit that referenced this issue Feb 2, 2023
refs: #15537

- snapshot test created to add confidence to webhook stability and increase overall test coverage.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue [triage] Start here if you've never contributed before. Hacktoberfest Issues suitable for hacktoberfest participants help wanted [triage] Ideal issues for contributors to help with
Projects
None yet
Development

No branches or pull requests