Skip to content

Commit

Permalink
Rework Activity in how it differentiates a JSON vs non-JSON response.
Browse files Browse the repository at this point in the history
Not all API responses are JSON, which can make it a bit awkward to
differentiate which kind of responses are JSON. This unifies the
two Activity variants so that there is only one, which simplifies
things a bit. Non-JSON responses are stored as a JSON string, under
the assumption that GitHub never response with a JSON string.
  • Loading branch information
ehuss committed Feb 20, 2023
1 parent 03a83c2 commit 7b98215
Show file tree
Hide file tree
Showing 53 changed files with 159 additions and 197 deletions.
86 changes: 31 additions & 55 deletions src/test_record.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,22 +27,21 @@ pub enum Activity {
webhook_event: String,
payload: serde_json::Value,
},
/// An outgoing request to api.github.com, and its response.
ApiRequest {
/// An outgoing request to api.github.com or raw.githubusercontent.com, and its response.
Request {
method: String,
path: String,
query: Option<String>,
request_body: String,
response_code: u16,
/// The body of the response.
///
/// For non-JSON requests, it is encoded as a `Value::String` under
/// the assumption that GitHub never returns a JSON string for a
/// response. This is done so that the JSON bodies can be formatted
/// nicely in the `.json` bodies to make inspecting them easier.
response_body: serde_json::Value,
},
/// An outgoing request to raw.githubusercontent.com, and its response.
RawRequest {
path: String,
query: Option<String>,
response_code: u16,
response_body: String,
},
/// Sent by the mock HTTP server to the test framework when it detects
/// something is wrong.
Error { message: String },
Expand All @@ -54,10 +53,8 @@ pub enum Activity {
/// Information about an HTTP request that is captured before sending.
///
/// This is needed to avoid messing with cloning the Request.
#[derive(Debug)]
pub struct RequestInfo {
/// If this is `true`, then it is for raw.githubusercontent.com.
/// If `false`, then it is for api.github.com.
is_raw: bool,
method: String,
path: String,
query: Option<String>,
Expand Down Expand Up @@ -175,9 +172,7 @@ pub fn capture_request(req: &Request) -> Option<RequestInfo> {
.and_then(|body| body.as_bytes())
.map(|bytes| String::from_utf8(bytes.to_vec()).unwrap())
.unwrap_or_default();
let is_raw = url.host_str().unwrap().contains("raw");
let info = RequestInfo {
is_raw,
method: req.method().to_string(),
path: url.path().to_string(),
query: url.query().map(|q| q.to_string()),
Expand All @@ -194,51 +189,32 @@ pub fn record_request(info: Option<RequestInfo>, status: StatusCode, body: &[u8]
let Some(info) = info else { return };
let Some(record_dir) = record_dir() else { return };
let response_code = status.as_u16();
let mut name = info.path.replace(['/', '.'], "_");
if name.starts_with('_') {
name.remove(0);
let mut munged_path = info.path.replace(['/', '.'], "_");
if munged_path.starts_with('_') {
munged_path.remove(0);
}
let (kind, activity) = if info.is_raw {
(
"raw",
Activity::RawRequest {
path: info.path,
query: info.query,
response_code,
response_body: String::from_utf8_lossy(body).to_string(),
},
)
let name = format!("{}-{}", info.method, munged_path);
// This is a hack to reduce the amount of data stored in the test
// directory. This file gets requested many times, and it is very
// large.
let response_body = if info.path == "/v1/teams.json" {
serde_json::json!(null)
} else {
let json_body = if info.path == "/v1/teams.json" {
// This is a hack to reduce the amount of data stored in the test
// directory. This file gets requested many times, and it is very
// large.
serde_json::json!({})
} else {
match serde_json::from_slice(body) {
Ok(json) => json,
Err(e) => {
error!("failed to record API response for {}: {e:?}", info.path);
return;
}
}
};
name.insert(0, '-');
name.insert_str(0, &info.method);
(
"api",
Activity::ApiRequest {
method: info.method,
path: info.path,
query: info.query,
request_body: info.body,
response_code,
response_body: json_body,
},
)
match serde_json::from_slice(body) {
Ok(json) => json,
Err(_) => serde_json::Value::String(String::from_utf8_lossy(body).to_string()),
}
};
let activity = Activity::Request {
method: info.method,
path: info.path,
query: info.query,
request_body: info.body,
response_code,
response_body,
};

let filename = format!("{:02}-{kind}-{name}.json", next_sequence());
let filename = format!("{:02}-{name}.json", next_sequence());
save_activity(&record_dir.join(filename), &activity);
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
{
"kind": "ApiRequest",
"kind": "Request",
"method": "GET",
"path": "/repos/rust-lang/rust/commits",
"query": "author=bors",
Expand Down Expand Up @@ -2527,4 +2527,4 @@
"url": "https://api.github.com/repos/rust-lang/rust/commits/7c4a9a971ca6962533bed01ffbd0c1f6b5250abc"
}
]
}
}
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
{
"kind": "ApiRequest",
"kind": "Request",
"method": "GET",
"path": "/repos/ehuss/rust",
"query": null,
Expand Down Expand Up @@ -362,4 +362,4 @@
"watchers_count": 0,
"web_commit_signoff_required": false
}
}
}
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
{
"kind": "ApiRequest",
"kind": "Request",
"method": "POST",
"path": "/repos/ehuss/rust/git/commits",
"query": null,
Expand Down Expand Up @@ -39,4 +39,4 @@
"verified": false
}
}
}
}
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
{
"kind": "ApiRequest",
"kind": "Request",
"method": "GET",
"path": "/repos/rust-lang/rust",
"query": null,
Expand Down Expand Up @@ -157,4 +157,4 @@
"watchers_count": 77402,
"web_commit_signoff_required": false
}
}
}
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
{
"kind": "ApiRequest",
"kind": "Request",
"method": "GET",
"path": "/repos/rust-lang/rust/issues",
"query": "labels=A-coherence&filter=all&sort=created&direction=asc&per_page=100",
Expand Down Expand Up @@ -428,4 +428,4 @@
}
}
]
}
}
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
{
"kind": "ApiRequest",
"kind": "Request",
"method": "GET",
"path": "/repos/rust-lang/rust",
"query": null,
Expand Down Expand Up @@ -157,4 +157,4 @@
"watchers_count": 77405,
"web_commit_signoff_required": false
}
}
}
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
{
"kind": "ApiRequest",
"kind": "Request",
"method": "GET",
"path": "/search/issues",
"query": "q=state:closed+is:pull-request+label:beta-nominated+label:beta-accepted+repo:rust-lang/rust&sort=created&order=asc&per_page=100&page=1",
Expand Down Expand Up @@ -611,4 +611,4 @@
],
"total_count": 3
}
}
}
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
{
"kind": "ApiRequest",
"kind": "Request",
"method": "GET",
"path": "/repos/rust-lang/rust",
"query": null,
Expand Down Expand Up @@ -157,4 +157,4 @@
"watchers_count": 77402,
"web_commit_signoff_required": false
}
}
}
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
{
"kind": "ApiRequest",
"kind": "Request",
"method": "GET",
"path": "/repos/rust-lang/rust/git/ref/heads/stable",
"query": null,
Expand All @@ -15,4 +15,4 @@
"ref": "refs/heads/stable",
"url": "https://api.github.com/repos/rust-lang/rust/git/refs/heads/stable"
}
}
}
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
{
"kind": "ApiRequest",
"kind": "Request",
"method": "GET",
"path": "/repos/rust-lang/rust",
"query": null,
Expand Down Expand Up @@ -145,4 +145,4 @@
"watchers_count": 77396,
"web_commit_signoff_required": false
}
}
}
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
{
"kind": "ApiRequest",
"kind": "Request",
"method": "GET",
"path": "/repos/rust-lang/rust/git/commits/109cccbe4f345c0f0785ce860788580c3e2a29f5",
"query": null,
Expand Down Expand Up @@ -44,4 +44,4 @@
"verified": false
}
}
}
}
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
{
"kind": "ApiRequest",
"kind": "Request",
"method": "GET",
"path": "/repos/rust-lang/rust",
"query": null,
Expand Down Expand Up @@ -145,4 +145,4 @@
"watchers_count": 77394,
"web_commit_signoff_required": false
}
}
}
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
{
"kind": "ApiRequest",
"kind": "Request",
"method": "GET",
"path": "/repos/rust-lang/rust/commits",
"query": "author=octocat",
"request_body": "",
"response_code": 200,
"response_body": []
}
}
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
{
"kind": "ApiRequest",
"kind": "Request",
"method": "GET",
"path": "/repos/rust-lang/rust/commits",
"query": "author=brson",
Expand Down Expand Up @@ -2377,4 +2377,4 @@
"url": "https://api.github.com/repos/rust-lang/rust/commits/2afadaadc99118b169d2c3aec01e9814409b37fa"
}
]
}
}
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
{
"kind": "ApiRequest",
"kind": "Request",
"method": "GET",
"path": "/repos/rust-lang/rust",
"query": null,
Expand Down Expand Up @@ -157,4 +157,4 @@
"watchers_count": 77402,
"web_commit_signoff_required": false
}
}
}
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
{
"kind": "ApiRequest",
"kind": "Request",
"method": "GET",
"path": "/repos/rust-lang/rust/issues",
"query": "labels=A-coherence&filter=all&sort=created&direction=asc&per_page=100",
Expand Down Expand Up @@ -428,4 +428,4 @@
}
}
]
}
}
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
{
"kind": "ApiRequest",
"kind": "Request",
"method": "GET",
"path": "/repos/ehuss/rust",
"query": null,
Expand Down Expand Up @@ -362,4 +362,4 @@
"watchers_count": 0,
"web_commit_signoff_required": false
}
}
}
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
{
"kind": "ApiRequest",
"kind": "Request",
"method": "POST",
"path": "/repos/ehuss/rust/merge-upstream",
"query": null,
Expand All @@ -10,4 +10,4 @@
"merge_type": "fast-forward",
"message": "Successfully fetched and fast-forwarded from upstream rust-lang:master."
}
}
}
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
{
"kind": "ApiRequest",
"kind": "Request",
"method": "GET",
"path": "/repos/ehuss/rust",
"query": null,
Expand Down Expand Up @@ -362,4 +362,4 @@
"watchers_count": 0,
"web_commit_signoff_required": false
}
}
}
Loading

0 comments on commit 7b98215

Please sign in to comment.