Skip to content

Commit 2e873da

Browse files
larscomlarscom
authored
306 feature request view unit tests and code coverage (#317)
* add coverage * add debug logging
1 parent 5d6e2a3 commit 2e873da

File tree

21 files changed

+304
-228
lines changed

21 files changed

+304
-228
lines changed

‎api/src/error.rs‎

Lines changed: 15 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -13,34 +13,40 @@ pub struct ApiError {
1313
}
1414

1515
impl ApiError {
16-
pub fn new(status_code: StatusCode, message: String) -> Self {
16+
pub fn new(status_code: StatusCode, message: impl Into<String>) -> Self {
1717
Self {
1818
status_code: status_code.as_u16(),
19-
message,
19+
message: message.into(),
2020
}
2121
}
2222

23+
pub fn server_error(message: impl Into<String>) -> Self {
24+
Self::new(StatusCode::INTERNAL_SERVER_ERROR, message)
25+
}
26+
27+
pub fn bad_request(message: impl Into<String>) -> Self {
28+
Self::new(StatusCode::BAD_REQUEST, message)
29+
}
30+
2331
pub fn with_u16_code(status_code: u16, message: String) -> Self {
2432
Self {
2533
status_code,
2634
message,
2735
}
2836
}
2937

30-
pub fn server_error(message: String) -> Self {
31-
Self::new(StatusCode::INTERNAL_SERVER_ERROR, message)
32-
}
33-
34-
pub fn bad_request(message: String) -> Self {
35-
Self::new(StatusCode::BAD_REQUEST, message)
38+
pub fn is_forbidden(&self) -> bool {
39+
StatusCode::from_u16(self.status_code)
40+
.map(|s| s == StatusCode::FORBIDDEN)
41+
.unwrap_or(false)
3642
}
3743
}
3844

3945
impl Default for ApiError {
4046
fn default() -> Self {
4147
Self::new(
4248
StatusCode::INTERNAL_SERVER_ERROR,
43-
"an internal server error occured".into(),
49+
"an internal server error occured",
4450
)
4551
}
4652
}

‎api/src/gitlab.rs‎

Lines changed: 40 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -101,7 +101,7 @@ impl GitlabClient {
101101
let url = Url::parse_with_params(format!("{}{}", self.base_url, path).as_str(), params)
102102
.expect("url to be parsed with params");
103103

104-
log::debug!("HTTP (post) {url} body: {body_json:?}");
104+
log::debug!("request (post) {url} body: {body_json:?}");
105105
let builder = self.http_client.post(url);
106106

107107
match body_json {
@@ -115,8 +115,16 @@ impl GitlabClient {
115115
path: String,
116116
params: Vec<(String, String)>,
117117
body_json: Option<Value>,
118-
) -> Result<T, reqwest::Error> {
119-
self.do_post(path, params, body_json).await?.json().await
118+
) -> Result<T, ApiError> {
119+
let response = self.do_post(path.clone(), params, body_json).await?;
120+
let json = response.text().await?;
121+
122+
log::debug!("path: {}, JSON response: {}", &path, &json);
123+
124+
let data =
125+
serde_json::from_str(&json).map_err(|e| ApiError::server_error(e.to_string()))?;
126+
127+
Ok(data)
120128
}
121129

122130
async fn do_get(
@@ -127,7 +135,7 @@ impl GitlabClient {
127135
let url = Url::parse_with_params(format!("{}{}", self.base_url, path).as_str(), params)
128136
.expect("url to be parsed with params");
129137

130-
log::debug!("HTTP (get) {url}");
138+
log::debug!("request (get) {url}");
131139

132140
self.http_client.get(url).send().await?.error_for_status()
133141
}
@@ -136,23 +144,34 @@ impl GitlabClient {
136144
&self,
137145
path: String,
138146
params: Vec<(String, String)>,
139-
) -> Result<T, reqwest::Error> {
140-
self.do_get(path, params).await?.json().await
147+
) -> Result<T, ApiError> {
148+
let response = self.do_get(path.clone(), params).await?;
149+
let json = response.text().await?;
150+
151+
log::debug!("path: {}, JSON response: {}", &path, &json);
152+
153+
let data =
154+
serde_json::from_str(&json).map_err(|e| ApiError::server_error(e.to_string()))?;
155+
Ok(data)
141156
}
142157

143158
async fn get_page<T: DeserializeOwned>(
144159
&self,
145160
path: String,
146161
page: usize,
147162
params: Vec<(String, String)>,
148-
) -> Result<Page<T>, reqwest::Error> {
163+
) -> Result<Page<T>, ApiError> {
149164
let mut params = params;
150165
params.push(("page".to_string(), page.to_string()));
151166
params.push(("per_page".to_string(), "100".to_string()));
152167

153-
let response = self.do_get(path, params).await?;
168+
let response = self.do_get(path.clone(), params).await?;
154169
let total_pages = get_total_pages(response.headers());
155-
let data = response.json().await?;
170+
let json = response.text().await?;
171+
172+
log::debug!("page: {}, path: {}, JSON response: {}", page, &path, &json);
173+
let data =
174+
serde_json::from_str(&json).map_err(|e| ApiError::server_error(e.to_string()))?;
156175

157176
Ok(Page {
158177
data,
@@ -170,15 +189,15 @@ impl GitlabClient {
170189
T: DeserializeOwned + Send + 'static,
171190
T: std::fmt::Debug,
172191
{
173-
log::debug!("fetching page 1");
192+
log::debug!("fetching page 1, path: {}", &path);
174193

175194
let Page {
176195
data: mut all_data,
177196
total_pages,
178197
page: _,
179198
} = self.get_page(path.clone(), 1, params.clone()).await?;
180199

181-
log::debug!("fetched page 1/{total_pages} data: {all_data:?}");
200+
log::debug!("fetched page 1/{total_pages}, path: {}", &path);
182201

183202
if total_pages == 1 {
184203
return Ok(all_data);
@@ -192,7 +211,7 @@ impl GitlabClient {
192211
let path = path.clone();
193212
let tx = tx.clone();
194213
tokio::spawn(async move {
195-
log::debug!("fetching page {page}");
214+
log::debug!("fetching page {page}, path: {}", &path);
196215
let result = self_clone.get_page(path, page, params).await;
197216
if let Err(err) = tx.send(result).await {
198217
log::error!("could not send result via channel. err: {err}");
@@ -208,7 +227,7 @@ impl GitlabClient {
208227
total_pages,
209228
page,
210229
} = result?;
211-
log::debug!("fetched page {page}/{total_pages} data: {data:?}");
230+
log::debug!("fetched page {page}/{total_pages}");
212231
all_data.append(&mut data);
213232
}
214233

@@ -263,17 +282,11 @@ impl GitlabApi for GitlabClient {
263282
match self.do_get_parsed::<Pipeline>(path, params.to_vec()).await {
264283
Ok(pipeline) => Ok(Some(pipeline)),
265284
Err(error) => {
266-
let status = error.status();
267-
status.map_or_else(
268-
|| Ok(None),
269-
|code| {
270-
if code == reqwest::StatusCode::FORBIDDEN {
271-
Ok(None)
272-
} else {
273-
Err(error.into())
274-
}
275-
},
276-
)
285+
if error.is_forbidden() {
286+
Ok(None)
287+
} else {
288+
Err(error)
289+
}
277290
}
278291
}
279292
}
@@ -299,9 +312,7 @@ impl GitlabApi for GitlabClient {
299312
let params = [];
300313
let path = format!("/projects/{project_id}/pipelines/{pipeline_id}/retry");
301314

302-
self.do_post_parsed(path, params.to_vec(), None)
303-
.await
304-
.map_err(|e| e.into())
315+
self.do_post_parsed(path, params.to_vec(), None).await
305316
}
306317

307318
async fn start_pipeline(
@@ -328,9 +339,7 @@ impl GitlabApi for GitlabClient {
328339
Value::Object(o)
329340
});
330341

331-
self.do_post_parsed(path, params.to_vec(), body_json)
332-
.await
333-
.map_err(|e| e.into())
342+
self.do_post_parsed(path, params.to_vec(), body_json).await
334343
}
335344

336345
async fn cancel_pipeline(
@@ -341,9 +350,7 @@ impl GitlabApi for GitlabClient {
341350
let params = [];
342351
let path = format!("/projects/{project_id}/pipelines/{pipeline_id}/cancel");
343352

344-
self.do_post_parsed(path, params.to_vec(), None)
345-
.await
346-
.map_err(|e| e.into())
353+
self.do_post_parsed(path, params.to_vec(), None).await
347354
}
348355

349356
async fn branches(&self, project_id: u64) -> Result<Vec<Branch>, ApiError> {

‎api/src/model/mod.rs‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,10 @@ pub mod user;
1919
pub mod test {
2020
use crate::model::commit::Commit;
2121
use crate::model::user::User;
22-
use crate::model::{Branch, Group, Job, JobStatus, Namespace, Pipeline, PipelineSource, PipelineStatus, Project, Schedule};
22+
use crate::model::{
23+
Branch, Group, Job, JobStatus, Namespace, Pipeline, PipelineSource, PipelineStatus,
24+
Project, Schedule,
25+
};
2326

2427
pub fn new_commit() -> Commit {
2528
Commit {
@@ -49,6 +52,7 @@ pub mod test {
4952
id: 1,
5053
iid: 2,
5154
project_id: 3,
55+
coverage: None,
5256
sha: "sha".to_string(),
5357
branch: "branch".to_string(),
5458
status: PipelineStatus::Running,
@@ -103,7 +107,7 @@ pub mod test {
103107
id: 123,
104108
name: "namespace".to_string(),
105109
path: "namespace".to_string(),
106-
}
110+
},
107111
}
108112
}
109113

‎api/src/model/pipeline.rs‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
use crate::util::deserialize::into_opt_f64;
12
use chrono::{DateTime, Utc};
23
use serde::{Deserialize, Serialize};
34

@@ -6,6 +7,12 @@ pub struct Pipeline {
67
pub id: u64,
78
pub iid: u64,
89
pub project_id: u64,
10+
#[serde(
11+
default,
12+
deserialize_with = "into_opt_f64",
13+
skip_serializing_if = "Option::is_none"
14+
)]
15+
pub coverage: Option<f64>,
916
pub sha: String,
1017
#[serde(rename = "ref")]
1118
pub branch: String,

‎api/src/pipeline/pipeline_handler.rs‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ async fn retry_pipeline(
4545
) -> Result<Json<Pipeline>, ApiError> {
4646
if api_config.read_only {
4747
return Err(ApiError::bad_request(
48-
"can't retry pipeline when in 'read only' mode".into(),
48+
"can't retry pipeline when in 'read only' mode",
4949
));
5050
}
5151

@@ -66,7 +66,7 @@ async fn cancel_pipeline(
6666
) -> Result<Json<Pipeline>, ApiError> {
6767
if api_config.read_only {
6868
return Err(ApiError::bad_request(
69-
"can't cancel pipeline when in 'read only' mode".into(),
69+
"can't cancel pipeline when in 'read only' mode",
7070
));
7171
}
7272

@@ -95,7 +95,7 @@ async fn start_pipeline(
9595
) -> Result<Json<Pipeline>, ApiError> {
9696
if api_config.read_only {
9797
return Err(ApiError::bad_request(
98-
"can't start a new pipeline when in 'read only' mode".into(),
98+
"can't start a new pipeline when in 'read only' mode",
9999
));
100100
}
101101

‎api/src/schedule/pipeline_aggregator.rs‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -78,15 +78,13 @@ impl PipelineAggregator {
7878
schedules: Vec<Schedule>,
7979
) -> Result<Vec<ScheduleProjectPipeline>, ApiError> {
8080
try_collect_with_buffer(schedules, |schedule| async move {
81-
let project = project.clone();
81+
let project = project.to_owned();
8282

8383
let pipeline = self
8484
.pipeline_service
8585
.get_latest_pipeline(project.id, schedule.branch.clone())
8686
.await?;
8787

88-
let schedule = schedule.clone();
89-
9088
let failed_jobs = match pipeline {
9189
Some(ref p) if p.status == PipelineStatus::Failed => Some(
9290
self.job_service

‎api/src/util/deserialize.rs‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,3 +7,13 @@ where
77
let value: String = Deserialize::deserialize(deserializer)?;
88
Ok(value.rsplit('/').next().map(|b| b.into()).unwrap_or(value))
99
}
10+
11+
pub fn into_opt_f64<'de, D>(deserializer: D) -> Result<Option<f64>, D::Error>
12+
where
13+
D: Deserializer<'de>,
14+
{
15+
match Option::<String>::deserialize(deserializer)? {
16+
Some(value) => value.parse().map(Some).map_err(serde::de::Error::custom),
17+
None => Ok(None),
18+
}
19+
}

‎src/app/groups/group-tabs/feature-tabs/components/write-actions-icon/start-pipeline-action/start-pipeline-modal/start-pipeline-modal.component.html‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@
1212
[(ngModel)]="selectedBranch"
1313
>
1414
<nz-option [nzLabel]="defaultBranch" [nzValue]="defaultBranch"></nz-option>
15-
@for (branch of branches(); track branch.name) {
15+
@for (branch of branches(); track branch.commit.id) {
1616
<nz-option [nzLabel]="branch.name" [nzValue]="branch.name"></nz-option>
1717
}
1818
</nz-select>

‎src/app/groups/group-tabs/feature-tabs/feature-tabs.component.html‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
nzSize="small"
77
nzType="card"
88
>
9-
@for (tab of tabs; track tab) {
9+
@for (tab of tabs; track tab.id) {
1010
<nz-tab [nzTitle]="title">
1111
<ng-template #title>
1212
<span nz-icon [nzType]="tab.icon"></span><span class="pointer">{{ tab.title }}</span>

‎src/app/groups/group-tabs/feature-tabs/latest-pipelines/pipeline-status-tabs/pipeline-table/pipeline-table-branch/pipeline-table-branch.component.html‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -37,15 +37,18 @@
3737
@for (data of nzTable.data; track trackByBranchCommitId(data)) {
3838
<tr [attr.data-pipeline-id]="data.pipeline?.id">
3939
<td>{{ data.branch.name }}</td>
40+
<td [style.color]="data.pipeline?.coverage | coverageColor">
41+
{{ data.pipeline?.coverage ? data.pipeline?.coverage + '%' : '-' }}
42+
</td>
43+
<td>{{ data.pipeline?.source || '-' }}</td>
4044
<td>
4145
@if (data.pipeline; as pipeline) {
42-
<nz-badge [nzColor]="pipeline.status | statusColor" [nzText]="pipeline.status"></nz-badge>
46+
{{ pipeline.updated_at | date : 'medium' : timeZone : locale }}
4347
} @else { - }
4448
</td>
45-
<td>{{ data.pipeline?.source || '-' }}</td>
4649
<td>
4750
@if (data.pipeline; as pipeline) {
48-
{{ pipeline.updated_at | date : 'medium' : timeZone : locale }}
51+
<nz-badge [nzColor]="pipeline.status | statusColor" [nzText]="pipeline.status"></nz-badge>
4952
} @else { - }
5053
</td>
5154
<td>

0 commit comments

Comments
 (0)