Skip to content

Commit 7b98215

Browse files
committed
Rework Activity in how it differentiates a JSON vs non-JSON response.
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.
1 parent 03a83c2 commit 7b98215

File tree

53 files changed

+159
-197
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

53 files changed

+159
-197
lines changed

src/test_record.rs

Lines changed: 31 additions & 55 deletions
Original file line numberDiff line numberDiff line change
@@ -27,22 +27,21 @@ pub enum Activity {
2727
webhook_event: String,
2828
payload: serde_json::Value,
2929
},
30-
/// An outgoing request to api.github.com, and its response.
31-
ApiRequest {
30+
/// An outgoing request to api.github.com or raw.githubusercontent.com, and its response.
31+
Request {
3232
method: String,
3333
path: String,
3434
query: Option<String>,
3535
request_body: String,
3636
response_code: u16,
37+
/// The body of the response.
38+
///
39+
/// For non-JSON requests, it is encoded as a `Value::String` under
40+
/// the assumption that GitHub never returns a JSON string for a
41+
/// response. This is done so that the JSON bodies can be formatted
42+
/// nicely in the `.json` bodies to make inspecting them easier.
3743
response_body: serde_json::Value,
3844
},
39-
/// An outgoing request to raw.githubusercontent.com, and its response.
40-
RawRequest {
41-
path: String,
42-
query: Option<String>,
43-
response_code: u16,
44-
response_body: String,
45-
},
4645
/// Sent by the mock HTTP server to the test framework when it detects
4746
/// something is wrong.
4847
Error { message: String },
@@ -54,10 +53,8 @@ pub enum Activity {
5453
/// Information about an HTTP request that is captured before sending.
5554
///
5655
/// This is needed to avoid messing with cloning the Request.
56+
#[derive(Debug)]
5757
pub struct RequestInfo {
58-
/// If this is `true`, then it is for raw.githubusercontent.com.
59-
/// If `false`, then it is for api.github.com.
60-
is_raw: bool,
6158
method: String,
6259
path: String,
6360
query: Option<String>,
@@ -175,9 +172,7 @@ pub fn capture_request(req: &Request) -> Option<RequestInfo> {
175172
.and_then(|body| body.as_bytes())
176173
.map(|bytes| String::from_utf8(bytes.to_vec()).unwrap())
177174
.unwrap_or_default();
178-
let is_raw = url.host_str().unwrap().contains("raw");
179175
let info = RequestInfo {
180-
is_raw,
181176
method: req.method().to_string(),
182177
path: url.path().to_string(),
183178
query: url.query().map(|q| q.to_string()),
@@ -194,51 +189,32 @@ pub fn record_request(info: Option<RequestInfo>, status: StatusCode, body: &[u8]
194189
let Some(info) = info else { return };
195190
let Some(record_dir) = record_dir() else { return };
196191
let response_code = status.as_u16();
197-
let mut name = info.path.replace(['/', '.'], "_");
198-
if name.starts_with('_') {
199-
name.remove(0);
192+
let mut munged_path = info.path.replace(['/', '.'], "_");
193+
if munged_path.starts_with('_') {
194+
munged_path.remove(0);
200195
}
201-
let (kind, activity) = if info.is_raw {
202-
(
203-
"raw",
204-
Activity::RawRequest {
205-
path: info.path,
206-
query: info.query,
207-
response_code,
208-
response_body: String::from_utf8_lossy(body).to_string(),
209-
},
210-
)
196+
let name = format!("{}-{}", info.method, munged_path);
197+
// This is a hack to reduce the amount of data stored in the test
198+
// directory. This file gets requested many times, and it is very
199+
// large.
200+
let response_body = if info.path == "/v1/teams.json" {
201+
serde_json::json!(null)
211202
} else {
212-
let json_body = if info.path == "/v1/teams.json" {
213-
// This is a hack to reduce the amount of data stored in the test
214-
// directory. This file gets requested many times, and it is very
215-
// large.
216-
serde_json::json!({})
217-
} else {
218-
match serde_json::from_slice(body) {
219-
Ok(json) => json,
220-
Err(e) => {
221-
error!("failed to record API response for {}: {e:?}", info.path);
222-
return;
223-
}
224-
}
225-
};
226-
name.insert(0, '-');
227-
name.insert_str(0, &info.method);
228-
(
229-
"api",
230-
Activity::ApiRequest {
231-
method: info.method,
232-
path: info.path,
233-
query: info.query,
234-
request_body: info.body,
235-
response_code,
236-
response_body: json_body,
237-
},
238-
)
203+
match serde_json::from_slice(body) {
204+
Ok(json) => json,
205+
Err(_) => serde_json::Value::String(String::from_utf8_lossy(body).to_string()),
206+
}
207+
};
208+
let activity = Activity::Request {
209+
method: info.method,
210+
path: info.path,
211+
query: info.query,
212+
request_body: info.body,
213+
response_code,
214+
response_body,
239215
};
240216

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

tests/github_client/bors_commits/00-api-GET-repos_rust-lang_rust_commits.json renamed to tests/github_client/bors_commits/00-GET-repos_rust-lang_rust_commits.json

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
{
2-
"kind": "ApiRequest",
2+
"kind": "Request",
33
"method": "GET",
44
"path": "/repos/rust-lang/rust/commits",
55
"query": "author=bors",
@@ -2527,4 +2527,4 @@
25272527
"url": "https://api.github.com/repos/rust-lang/rust/commits/7c4a9a971ca6962533bed01ffbd0c1f6b5250abc"
25282528
}
25292529
]
2530-
}
2530+
}

tests/github_client/update_tree/00-api-GET-repos_ehuss_rust.json renamed to tests/github_client/create_commit/00-GET-repos_ehuss_rust.json

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
{
2-
"kind": "ApiRequest",
2+
"kind": "Request",
33
"method": "GET",
44
"path": "/repos/ehuss/rust",
55
"query": null,
@@ -362,4 +362,4 @@
362362
"watchers_count": 0,
363363
"web_commit_signoff_required": false
364364
}
365-
}
365+
}

tests/github_client/create_commit/01-api-POST-repos_ehuss_rust_git_commits.json renamed to tests/github_client/create_commit/01-POST-repos_ehuss_rust_git_commits.json

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
{
2-
"kind": "ApiRequest",
2+
"kind": "Request",
33
"method": "POST",
44
"path": "/repos/ehuss/rust/git/commits",
55
"query": null,
@@ -39,4 +39,4 @@
3939
"verified": false
4040
}
4141
}
42-
}
42+
}

tests/github_client/submodule/00-api-GET-repos_rust-lang_rust.json renamed to tests/github_client/get_issues_no_search/00-GET-repos_rust-lang_rust.json

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
{
2-
"kind": "ApiRequest",
2+
"kind": "Request",
33
"method": "GET",
44
"path": "/repos/rust-lang/rust",
55
"query": null,
@@ -157,4 +157,4 @@
157157
"watchers_count": 77402,
158158
"web_commit_signoff_required": false
159159
}
160-
}
160+
}

tests/github_client/get_issues_no_search/01-api-GET-repos_rust-lang_rust_issues.json renamed to tests/github_client/get_issues_no_search/01-GET-repos_rust-lang_rust_issues.json

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
{
2-
"kind": "ApiRequest",
2+
"kind": "Request",
33
"method": "GET",
44
"path": "/repos/rust-lang/rust/issues",
55
"query": "labels=A-coherence&filter=all&sort=created&direction=asc&per_page=100",
@@ -428,4 +428,4 @@
428428
}
429429
}
430430
]
431-
}
431+
}

tests/github_client/get_issues_with_search/00-api-GET-repos_rust-lang_rust.json renamed to tests/github_client/get_issues_with_search/00-GET-repos_rust-lang_rust.json

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
{
2-
"kind": "ApiRequest",
2+
"kind": "Request",
33
"method": "GET",
44
"path": "/repos/rust-lang/rust",
55
"query": null,
@@ -157,4 +157,4 @@
157157
"watchers_count": 77405,
158158
"web_commit_signoff_required": false
159159
}
160-
}
160+
}

tests/github_client/get_issues_with_search/01-api-GET-search_issues.json renamed to tests/github_client/get_issues_with_search/01-GET-search_issues.json

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
{
2-
"kind": "ApiRequest",
2+
"kind": "Request",
33
"method": "GET",
44
"path": "/search/issues",
55
"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",
@@ -611,4 +611,4 @@
611611
],
612612
"total_count": 3
613613
}
614-
}
614+
}

tests/github_client/get_reference/00-api-GET-repos_rust-lang_rust.json renamed to tests/github_client/get_reference/00-GET-repos_rust-lang_rust.json

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
{
2-
"kind": "ApiRequest",
2+
"kind": "Request",
33
"method": "GET",
44
"path": "/repos/rust-lang/rust",
55
"query": null,
@@ -157,4 +157,4 @@
157157
"watchers_count": 77402,
158158
"web_commit_signoff_required": false
159159
}
160-
}
160+
}

tests/github_client/get_reference/01-api-GET-repos_rust-lang_rust_git_ref_heads_stable.json renamed to tests/github_client/get_reference/01-GET-repos_rust-lang_rust_git_ref_heads_stable.json

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
{
2-
"kind": "ApiRequest",
2+
"kind": "Request",
33
"method": "GET",
44
"path": "/repos/rust-lang/rust/git/ref/heads/stable",
55
"query": null,
@@ -15,4 +15,4 @@
1515
"ref": "refs/heads/stable",
1616
"url": "https://api.github.com/repos/rust-lang/rust/git/refs/heads/stable"
1717
}
18-
}
18+
}

0 commit comments

Comments
 (0)