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

Added e2e tests for page.unscheduled webhook #15722

Merged
merged 1 commit into from
Feb 2, 2023

Conversation

RobinCsl
Copy link
Contributor

Contributes to this issue: #15537

Got some code for us? Awesome 🎊!

Please include a description of your change & check your PR against this list, thanks!

  • There's a clear use-case for this code change, explained below
  • Commit message has a short title & references relevant issues
  • The build will pass (run yarn test:all and yarn 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

@RobinCsl RobinCsl changed the title Added e2e tests for page.unscheduled webhook Added e2e tests for page.unscheduled webhook (#15537) Oct 29, 2022
@naz naz self-assigned this Oct 31, 2022
@codecov
Copy link

codecov bot commented Oct 31, 2022

Codecov Report

Base: 72.07% // Head: 72.07% // Increases project coverage by +0.00% 🎉

Coverage data is based on head (ceaeeb2) compared to base (5ab7654).
Patch has no changes to coverable lines.

Additional details and impacted files
@@           Coverage Diff           @@
##             main   #15722   +/-   ##
=======================================
  Coverage   72.07%   72.07%           
=======================================
  Files        1740     1740           
  Lines      110387   110387           
  Branches    16646    16643    -3     
=======================================
+ Hits        79558    79565    +7     
+ Misses      29520    29515    -5     
+ Partials     1309     1307    -2     
Flag Coverage Δ
e2e-tests 85.42% <ø> (-0.03%) ⬇️
unit-tests 55.67% <ø> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
...b/tables/members-stripe-customers-subscriptions.js 95.83% <0.00%> (-4.17%) ⬇️
ghost/admin/app/components/gh-file-uploader.js 83.07% <0.00%> (-3.08%) ⬇️
ghost/data-generator/lib/tables/subscriptions.js 92.30% <0.00%> (-3.08%) ⬇️
ghost/core/core/app.js 80.00% <0.00%> (-2.50%) ⬇️
...host/data-generator/lib/tables/members-products.js 97.14% <0.00%> (ø)
ghost/data-generator/lib/tables/posts.js 91.04% <0.00%> (+1.49%) ⬆️
ghost/data-generator/lib/tables/members.js 100.00% <0.00%> (+1.51%) ⬆️
ghost/admin/app/components/gh-image-uploader.js 87.07% <0.00%> (+2.72%) ⬆️
.../data-generator/lib/tables/members-login-events.js 100.00% <0.00%> (+9.09%) ⬆️
...tor/lib/tables/members-paid-subscription-events.js 100.00% <0.00%> (+10.25%) ⬆️
... and 1 more

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.
📢 Do you have feedback about the report comment? Let us know in this issue.

Copy link
Contributor

@naz naz left a comment

Choose a reason for hiding this comment

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

Hey @RobinCsl! The changes are great, just need to fix the linting error. Otherwise the code is good to land in the main branch. Thank a lot for the contribution 😊

@github-actions
Copy link
Contributor

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 🙏

@RobinCsl RobinCsl requested a review from naz October 31, 2022 08:57
@RobinCsl
Copy link
Contributor Author

Hey @naz! Thanks a lot! :) I fixed the linting issues and merge conflicts. Could you please take another look? Not sure about your flow, but will you squash the commits automatically or I shall do it?

@RobinCsl
Copy link
Contributor Author

Hello again, @naz, would you mind setting the PRs I created as hacktoberfest-accepted while we iron out the last details for them to be merged? I would love for them to count toward my Hacktoberfest contribution :) Many thanks!

@ErisDS ErisDS mentioned this pull request Nov 2, 2022
29 tasks
Copy link
Contributor

@naz naz left a comment

Choose a reason for hiding this comment

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

Great job @RobinCsl ☺️ Thanks for polishing up the code so nicely 🙌

@ErisDS ErisDS assigned ErisDS and unassigned naz Jan 17, 2023
@ErisDS ErisDS force-pushed the rcsl/pages.unscheduled branch 4 times, most recently from 681f683 to ea90807 Compare January 30, 2023 11:22
@RobinCsl RobinCsl force-pushed the rcsl/pages.unscheduled branch from ea90807 to ceaeeb2 Compare January 31, 2023 07:27
@RobinCsl
Copy link
Contributor Author

Hey @ErisDS, I just rebased the branch on main, it should be all good now 🤞

@ErisDS ErisDS changed the title Added e2e tests for page.unscheduled webhook (#15537) Added e2e tests for page.unscheduled webhook Feb 2, 2023
@ErisDS ErisDS merged commit 1c019a3 into TryGhost:main Feb 2, 2023
@ErisDS
Copy link
Member

ErisDS commented Feb 2, 2023

Hey @RobinCsl thanks for that! I rebased it a week or more ago as I didn't wanna ask you to do it after taking soooo long to get to it 🙈, but have been butting against broken tests in main meaning it never went green!

Thankfully you caught it at a good moment and got the ✅

This is the very last one of these, thank you so much 🙌 🙌 🙌 🙌

And sorry again for being so slooooow 🙈

@RobinCsl
Copy link
Contributor Author

RobinCsl commented Feb 2, 2023

No worries at all, glad I could help. 🙂

naz pushed a commit to naz/Ghost that referenced this pull request Feb 9, 2023
refs: TryGhost#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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants