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.

Sign up for GitHub

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

Jump to bottom

Create testsuite for GithubClient #1698

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Create testsuite for GithubClient #1698

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
2b9c820
Make it so that the API URLs are configurable for testing
ehuss Jun 7, 2023
44b0496
Create testsuite for GithubClient.
ehuss Jun 7, 2023
9f14128
Add test for merge_upstream conflict.
ehuss Jun 12, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 15 additions & 0 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,21 @@ You need to sign up for a free account, and also deal with configuring the GitHu
* Secret: Enter a shared secret (some longish random text)
* Events: "Send me everything"

## Tests
Copy link
Contributor

Choose a reason for hiding this comment

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

@ehuss by reading this README snippet I don't easily understand at a glance how the testsuite is meant to be used and maintained. Is the documentation actually meant to be in tests/github_client/mod.rs as rustdoc's comments?

A few questions ahead. Sorry for the perhaps vague comments 🙂, I'm trying to squint and understand these changes and their mainteinance cost for future other contributors:

  • What is needed to test a new handler under ./src/handlers (which is something I'm currently doing)? I assume little to nothing if the new handler is just using the same Github API already tested, right?

  • (echoing the comment from Mark) What is needed to do to update tests? Say, we change a little something in how we interpret Github responses in some structs under ./src/github.rs?

  • is there a way to regenerate these JSON files automatically?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is the documentation actually meant to be in tests/github_client/mod.rs as rustdoc's comments?

I'm not quite sure I understand the question. The documentation in README.md is intended as a high-level overview of testing of triagebot, and includes a link to tests/github_client/mod.rs to read more details of that specific test suite. If and when more suites are added, they could be linked here, too.

What is needed to test a new handler under ./src/handlers (which is something I'm currently doing)? I assume little to nothing if the new handler is just using the same Github API already tested, right?

Right, handler testing isn't part of this PR. Testing of handlers is implemented in #1678, but I'm finding them to be even harder to wield than these tests, so I broke out just the github side of things to try to make some progress.

What is needed to do to update tests? Say, we change a little something in how we interpret Github responses in some structs under ./src/github.rs?

As mentioned above. The general process is to point the test at one of your personal repos, and re-record the live data.

is there a way to regenerate these JSON files automatically?

Also mentioned above. I can take a look at making it easier to regenerate them.


When possible, writing unittests is very helpful and one of the easiest ways to test.
For more advanced testing, there is an integration test called `testsuite` which provides a testing environment for testing triagebot.
At this time, there is one part to it:

* [`github_client`](tests/github_client/mod.rs) — Tests specifically targeting `GithubClient`.
This sets up an HTTP server that mimics api.github.com and verifies the client's behavior.

Other parts may be added in the future, such as testing the database or the triagebot server itself.

The real GitHub API responses are recorded in JSON files that the tests can later replay to verify the behavior of triagebot.
These recordings are enabled with the `TRIAGEBOT_TEST_RECORD_DIR` environment variable.
See the documentation in `github_client` for the steps for setting up recording to write a test.

## License

Triagebot is distributed under the terms of both the MIT license and the
Expand Down
3 changes: 1 addition & 2 deletions src/actions.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ use std::collections::HashMap;
use std::sync::Arc;

use async_trait::async_trait;
use reqwest::Client;
use serde::{Deserialize, Serialize};
use tera::{Context, Tera};

Expand Down Expand Up @@ -87,7 +86,7 @@ pub fn to_human(d: DateTime<Utc>) -> String {
#[async_trait]
impl<'a> Action for Step<'a> {
async fn call(&self) -> anyhow::Result<String> {
let gh = GithubClient::new_with_default_token(Client::new());
let gh = GithubClient::new_from_env();

// retrieve all Rust compiler meetings
// from today for 7 days
Expand Down
Loading