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

Testing for gh.py #5

Merged
merged 10 commits into from
May 28, 2024
Merged

Testing for gh.py #5

merged 10 commits into from
May 28, 2024

Conversation

ethanholz
Copy link
Contributor

This PR adds testing for gh.py making heavy use of mocks for the GitHub API calls. Additionally, this PR uses urljoin to help fix some of the testing issues that can occur with URLs.

This commit simplifies the parameter needed for calling _do_request.
Using urljoin allows for a user to either provide "/testing/route" or
"testing/route" and they will function the same in the context of
building the API. This is in an effort to simplify testing.
This commit adds the initial support for testing the GitHub portion of
this project. Most of this code is done via mocking all the HTTP
requests made to the GitHub API.
Removed the returned function call that is being mocked since we were
not using it.
Adds a test to validate that the wait call runs accordingly.
This simplifies and consolidates the test_remove_runner tests into a
single parameter driven test.
This consolidates and simplifies the do_request testing.
This simplifies and consolidates the create_runner_token tests to be
simpler.
Moved to a top level import to reduce repetition and simplify import
logic.
Copy link

codecov bot commented May 10, 2024

Welcome to Codecov 🎉

Once you merge this PR into your default branch, you're all set! Codecov will compare coverage reports and display results in all future pull requests.

Thanks for integrating Codecov - We've got you covered ☂️

@ethanholz ethanholz requested a review from dwhswenson May 10, 2024 18:20
Copy link
Contributor

@dwhswenson dwhswenson left a comment

Choose a reason for hiding this comment

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

Pretty good start.

  1. Add smoke tests, where possible, to warn us if PyGithub's API changes. Since you test our logic in the existing tests, those tests can be just smoke tests.
  2. Looks like a TODO still to be implemented (which might simplify the associated test, as well?)

tests/test_gh.py Outdated Show resolved Hide resolved
tests/test_gh.py Outdated Show resolved Hide resolved
Updated get_runner test to ensure that we could check the instance
correctly.
Copy link
Contributor

@dwhswenson dwhswenson left a comment

Choose a reason for hiding this comment

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

Looks good for now! Note that we might come back and make a few updates here and in gh.py (I noticed a camelCase variable name there, which should be changed).

Merging.

@dwhswenson dwhswenson merged commit 0f8c1eb into omsf-eco-infra:main May 28, 2024
5 checks passed
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.

2 participants