-
-
Notifications
You must be signed in to change notification settings - Fork 10.5k
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
Added e2e tests for page.tag.detached
webhook
#15651
Conversation
@@ -56,13 +68,47 @@ const buildPageSnapshotWithTiers = ({ | |||
}; | |||
}; | |||
|
|||
const buildPageSnapshotWithTiersAndTags = ({ |
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 it make sense to reuse the buildPageSnapshotWithTiers
method here?
Codecov ReportBase: 53.41% // Head: 53.41% // Increases project coverage by
Additional details and impacted files@@ Coverage Diff @@
## main #15651 +/- ##
=======================================
Coverage 53.41% 53.41%
=======================================
Files 1502 1502
Lines 97876 97876
Branches 10961 10962 +1
=======================================
+ Hits 52277 52282 +5
+ Misses 44342 44338 -4
+ Partials 1257 1256 -1
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
const tagSnapshot = { | ||
created_at: anyISODateTime, | ||
id: anyObjectId, | ||
name: anyString, |
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.
The tag's name, slug, visibility etc. are not changing between test runs? If they are not, would be best to have snapshot capture them as is - no need to include non-dynamic properties in the snapshot matcher object 👍
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.
Hey-hey @Dark-Knight11 👋 These are great changes are I think we are 90% there. I've left couple of comments around areas that need improvement. Feel free to reach out if you need more guidance and thanks for the contribution!
Note from our bot: Some changes have been requested on this pull request. Updating your code is great, but won't notify us, so please leave a comment so that we (and our bot) can see when you've made the changes. Thank you 🙏 |
Hi, I have made the requested changes kindly look into it and lmk if anymore changes are needed from my side. |
The snapshots seem to be outdated on and are failing now. Can you please re-record them again @Dark-Knight11 🙏 |
d696f78
to
8a3c40c
Compare
8a3c40c
to
9720459
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 on these changes @Dark-Knight11! Looking forward for more contributions 😊
0387210
to
9faac51
Compare
bb5375e
to
09d89e2
Compare
9b7446f
to
73ec31a
Compare
b4b78a6
to
fd25388
Compare
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
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
fd25388
to
d57cc8d
Compare
page.tag.detached
webhook
Hey @Dark-Knight11 thanks again for your contributions! This has been merged and the Please keep them coming 🙏 we have so much we want to achieve beyond hacktoberfest 😬 |
refs: #15537
This adds an e2e test and test snapshot for the
page.tag.detached
webhook so we can prevent regressions and bugs in the futureGot some code for us? Awesome 🎊!
Please include a description of your change & check your PR against this list, thanks!
yarn test:all
andyarn lint
)We appreciate your contribution!
Also, if you'd be interested in writing code like this for us more regularly, we're hiring:
https://careers.ghost.org