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

test: refactor probot client to be easier to test #83

Merged
merged 4 commits into from
Jul 2, 2021

Conversation

ckerr
Copy link
Member

@ckerr ckerr commented Jul 2, 2021

Take a stab at trying to improve the probot unit testing.

What's New

  • The probot client's state is now held in a ProbotClient class. This lets tests instantiate a client, use it, and inspect its state.
  • Use @octokit/webhooks-types to add octokit payload type safety and. Use type-fest's PartialDeep<> for building partial payloads safely. Your IDE will be happy!
  • One grey lining: this PR is mostly groundwork for unblocking other tests. By itself, it only moves coverage up about 3%.

Context

I'm PRing this now since it's at a good stopping point and @erickzhao, who is back tomorrow, has expressed interest in working on the bot. So (a) I wanted to get his opinion on whether this looks like progress, and (b) want to hand over the baton so that we're not both hacking the same code. My goal is to use this for integration tests between the bot & the broker.

- The probot client's state is now held in a ProbotClient class.
  This class can be instantiated in the spec so that it can be inspected.

- Setting info and Probot can now be injected from a spec via the
  ProbotClient constructor

- use @octokit/webhooks-types for octokit payload type safety and
  use type-fest for PartialDeep<> for building partial payloads safely.
@ckerr ckerr requested a review from erickzhao July 2, 2021 04:41
@lgtm-com
Copy link

lgtm-com bot commented Jul 2, 2021

This pull request introduces 1 alert when merging 90b734f into 6fc53a9 - view on LGTM.com

new alerts:

  • 1 for Missing await

ckerr added 2 commits July 2, 2021 09:36
eslint/no-unused-vars is superceded by the sibling rule in typescript-eslint
@ckerr ckerr merged commit 29406cb into main Jul 2, 2021
@ckerr ckerr deleted the bot/tests-with-types branch July 2, 2021 19:28
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.

None yet

2 participants