Skip to content

Commit e057569

Browse files
committed
Fix some tests
1 parent 2078f0c commit e057569

File tree

7 files changed

+154
-45
lines changed

7 files changed

+154
-45
lines changed

src/cli/src/dispatch.rs

+6-5
Original file line numberDiff line numberDiff line change
@@ -564,9 +564,8 @@ pub async fn diff(
564564
)
565565
};
566566

567-
if is_remote {
568-
let result = command::remote::diff(&repository, revision_1, &file_1).await?;
569-
println!("{result}");
567+
let result = if is_remote {
568+
command::remote::diff(&repository, revision_1, &file_1).await?
570569
} else {
571570
command::compare(
572571
compare_strategy,
@@ -576,8 +575,10 @@ pub async fn diff(
576575
keys,
577576
targets,
578577
output,
579-
)?;
580-
}
578+
)?
579+
};
580+
581+
println!("{result}");
581582

582583
Ok(())
583584
}

src/lib/src/api/local/compare.rs

+27-23
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,6 @@ pub fn compare_files(
6767
Ok(CompareResult::Tabular(result))
6868
} else if is_files_utf8(&file_1, &file_2) {
6969
let result = utf8_compare::compare(&file_1, &file_2)?;
70-
println!("{result}");
7170

7271
Ok(CompareResult::Text(result))
7372
} else {
@@ -90,7 +89,7 @@ fn compare_tabular(
9089
keys: Vec<String>,
9190
targets: Vec<String>,
9291
output: Option<PathBuf>,
93-
) -> Result<CompareTabular, OxenError> {
92+
) -> Result<(CompareTabular, String), OxenError> {
9493
let df_1 = tabular::read_df(file_1, DFOpts::empty())?;
9594
let df_2 = tabular::read_df(file_2, DFOpts::empty())?;
9695

@@ -116,7 +115,6 @@ fn compare_tabular(
116115
compare_id,
117116
);
118117

119-
maybe_print_compare_output(&strategy, &compare_tabular_raw);
120118
maybe_save_compare_output(&strategy, &mut compare_tabular_raw, output)?;
121119
maybe_write_cache(
122120
repo,
@@ -126,7 +124,8 @@ fn compare_tabular(
126124
&mut compare_tabular_raw,
127125
)?;
128126

129-
Ok(compare)
127+
let compare_string = compare_to_string(&strategy, &compare_tabular_raw);
128+
Ok((compare, compare_string))
130129
}
131130

132131
pub fn get_cached_compare(
@@ -637,35 +636,40 @@ fn maybe_write_cache(
637636
Ok(())
638637
}
639638

640-
fn maybe_print_compare_output(strategy: &CompareStrategy, compare_tabular_raw: &CompareTabularRaw) {
639+
fn compare_to_string(
640+
strategy: &CompareStrategy,
641+
compare_tabular_raw: &CompareTabularRaw,
642+
) -> String {
641643
let left_only_df = &compare_tabular_raw.left_only_df;
642644
let right_only_df = &compare_tabular_raw.right_only_df;
643645
let diff_df = &compare_tabular_raw.diff_df;
644646
let match_df = &compare_tabular_raw.match_df;
645647

648+
let mut results: Vec<String> = vec![];
649+
646650
match strategy {
647651
CompareStrategy::Hash => {
648-
println!("Added rows");
649-
println!("{:?}", left_only_df);
650-
651-
println!("Removed rows");
652-
println!("{:?}", right_only_df);
652+
results.push(format!("Added Rows\n\n{left_only_df}\n\n"));
653+
results.push(format!("Removed Rows\n\n{right_only_df}\n\n"));
653654
}
654655

655656
CompareStrategy::Join => {
656-
println!("Rows with matching keys and DIFFERENT targets");
657-
println!("{:?}", diff_df);
658-
659-
println!("Rows with matching keys and SAME targets");
660-
println!("{:?}", match_df);
661-
662-
println!("Rows with keys only in LEFT DataFrame");
663-
println!("{:?}", left_only_df);
664-
665-
println!("Rows with keys only in RIGHT DataFrame");
666-
println!("{:?}", right_only_df);
657+
results.push(format!(
658+
"Rows with matching keys and DIFFERENT targets\n\n{diff_df}\n\n"
659+
));
660+
results.push(format!(
661+
"Rows with matching keys and SAME targets\n\n{match_df}\n\n"
662+
));
663+
results.push(format!(
664+
"Rows with keys only in LEFT DataFrame\n\n{left_only_df}\n\n"
665+
));
666+
results.push(format!(
667+
"Rows with keys only in RIGHT DataFrame\n\n{right_only_df}\n\n"
668+
));
667669
}
668670
}
671+
672+
results.join("\n")
669673
}
670674

671675
fn maybe_save_compare_output(
@@ -805,7 +809,7 @@ mod tests {
805809
None,
806810
)?;
807811

808-
if let CompareResult::Tabular(compare) = result {
812+
if let CompareResult::Tabular((compare, _)) = result {
809813
// Should be updated values
810814
assert_eq!(compare.derived["left_only"].size.height, 2);
811815
assert_eq!(compare.derived["right_only"].size.height, 1);
@@ -931,7 +935,7 @@ mod tests {
931935
None,
932936
)?;
933937

934-
if let CompareResult::Tabular(new_compare) = result {
938+
if let CompareResult::Tabular((new_compare, _)) = result {
935939
// Should be updated values
936940
assert_eq!(new_compare.derived["left_only"].size.height, 0);
937941
assert_eq!(new_compare.derived["right_only"].size.height, 6);

src/lib/src/command/compare.rs

+10-3
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ use crate::api::local::compare::CompareStrategy;
33
use crate::error::OxenError;
44
use crate::model::entry::commit_entry::{CommitPath, CompareEntry};
55
use crate::model::LocalRepository;
6+
use crate::view::compare::CompareResult;
67
use std::path::PathBuf;
78

89
pub fn compare(
@@ -13,7 +14,7 @@ pub fn compare(
1314
keys: Vec<String>,
1415
targets: Vec<String>,
1516
output: Option<PathBuf>,
16-
) -> Result<(), OxenError> {
17+
) -> Result<String, OxenError> {
1718
let mut compare_entry_1 = CompareEntry {
1819
commit_entry: None,
1920
path: cpath_1.path.clone(),
@@ -46,7 +47,7 @@ pub fn compare(
4647
compare_entry_2.commit_entry = Some(entry_2);
4748
};
4849

49-
api::local::compare::compare_files(
50+
let compare_result = api::local::compare::compare_files(
5051
compare_strategy,
5152
repo,
5253
None,
@@ -56,5 +57,11 @@ pub fn compare(
5657
targets,
5758
output,
5859
)?;
59-
Ok(())
60+
61+
let text = match compare_result {
62+
CompareResult::Tabular((_, text)) => text,
63+
CompareResult::Text(text) => text,
64+
};
65+
66+
Ok(text)
6067
}

src/lib/src/view/compare.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,7 @@ pub struct CompareTabular {
7474

7575
#[derive(Debug)]
7676
pub enum CompareResult {
77-
Tabular(CompareTabular),
77+
Tabular((CompareTabular, String)),
7878
Text(String),
7979
}
8080

src/server/src/controllers/compare.rs

+97-4
Original file line numberDiff line numberDiff line change
@@ -8,11 +8,12 @@ use liboxen::core::index::{CommitReader, Merger};
88
use liboxen::error::OxenError;
99
use liboxen::message::OxenMessage;
1010
use liboxen::model::compare::tabular_compare::TabularCompareBody;
11-
use liboxen::model::{DataFrameSize, Schema};
11+
use liboxen::model::{Commit, DataFrameSize, LocalRepository, Schema};
1212
use liboxen::opts::df_opts::DFOptsView;
1313
use liboxen::opts::DFOpts;
1414
use liboxen::view::compare::{
15-
CompareCommits, CompareCommitsResponse, CompareEntries, CompareResult, CompareTabularResponse,
15+
CompareCommits, CompareCommitsResponse, CompareEntries, CompareEntryResponse, CompareResult,
16+
CompareTabularResponse,
1617
};
1718
use liboxen::view::json_data_frame_view::{DFResourceType, DerivedDFResource, JsonDataFrameSource};
1819
use liboxen::view::{
@@ -121,6 +122,52 @@ pub async fn entries(
121122
Ok(HttpResponse::Ok().json(view))
122123
}
123124

125+
pub async fn file(
126+
req: HttpRequest,
127+
query: web::Query<DFOptsQuery>,
128+
) -> actix_web::Result<HttpResponse, OxenHttpError> {
129+
let app_data = app_data(&req)?;
130+
let namespace = path_param(&req, "namespace")?;
131+
let name = path_param(&req, "repo_name")?;
132+
let base_head = path_param(&req, "base_head")?;
133+
134+
// Get the repository or return error
135+
let repository = get_repo(&app_data.path, namespace, name)?;
136+
137+
// Parse the base and head from the base..head/resource string
138+
// For Example)
139+
// main..feature/add-data/path/to/file.txt
140+
let (base_commit, head_commit, resource) = parse_base_head_resource(&repository, &base_head)?;
141+
142+
let base_entry = api::local::entries::get_commit_entry(&repository, &base_commit, &resource)?;
143+
let head_entry = api::local::entries::get_commit_entry(&repository, &head_commit, &resource)?;
144+
145+
let mut opts = DFOpts::empty();
146+
opts = df_opts_query::parse_opts(&query, &mut opts);
147+
148+
let page_size = query.page_size.unwrap_or(constants::DEFAULT_PAGE_SIZE);
149+
let page = query.page.unwrap_or(constants::DEFAULT_PAGE_NUM);
150+
151+
let start = if page == 0 { 0 } else { page_size * (page - 1) };
152+
let end = page_size * page;
153+
opts.slice = Some(format!("{}..{}", start, end));
154+
155+
let diff = api::local::diff::diff_entries(
156+
&repository,
157+
base_entry,
158+
&base_commit,
159+
head_entry,
160+
&head_commit,
161+
opts,
162+
)?;
163+
164+
let view = CompareEntryResponse {
165+
status: StatusMessage::resource_found(),
166+
compare: diff,
167+
};
168+
Ok(HttpResponse::Ok().json(view))
169+
}
170+
124171
pub async fn create_df_compare(
125172
req: HttpRequest,
126173
_query: web::Query<DFOptsQuery>,
@@ -191,7 +238,7 @@ pub async fn create_df_compare(
191238
)?;
192239

193240
let view = match result {
194-
CompareResult::Tabular(compare) => {
241+
CompareResult::Tabular((compare, _)) => {
195242
let mut messages: Vec<OxenMessage> = vec![];
196243

197244
if compare.dupes.left > 0 || compare.dupes.right > 0 {
@@ -297,7 +344,7 @@ pub async fn get_df_compare(
297344
)?;
298345

299346
match result {
300-
CompareResult::Tabular(compare) => {
347+
CompareResult::Tabular((compare, _)) => {
301348
let mut messages: Vec<OxenMessage> = vec![];
302349

303350
if compare.dupes.left > 0 || compare.dupes.right > 0 {
@@ -442,3 +489,49 @@ pub async fn get_derived_df(
442489
}
443490
}
444491
}
492+
493+
fn parse_base_head_resource(
494+
repo: &LocalRepository,
495+
base_head: &str,
496+
) -> Result<(Commit, Commit, PathBuf), OxenError> {
497+
log::debug!("Parsing base_head_resource: {}", base_head);
498+
499+
let mut split = base_head.split("..");
500+
let base = split
501+
.next()
502+
.ok_or(OxenError::resource_not_found(base_head))?;
503+
let head = split
504+
.next()
505+
.ok_or(OxenError::resource_not_found(base_head))?;
506+
507+
let base_commit = api::local::revisions::get(repo, base)?
508+
.ok_or(OxenError::revision_not_found(base.into()))?;
509+
510+
// Split on / and find longest branch name
511+
let split_head = head.split('/');
512+
let mut longest_str = String::from("");
513+
let mut head_commit: Option<Commit> = None;
514+
let mut resource: Option<PathBuf> = None;
515+
516+
for s in split_head {
517+
let maybe_revision = format!("{}{}", longest_str, s);
518+
log::debug!("Checking maybe head revision: {}", maybe_revision);
519+
let commit = api::local::revisions::get(repo, &maybe_revision)?;
520+
if commit.is_some() {
521+
head_commit = commit;
522+
let mut r_str = head.replace(&maybe_revision, "");
523+
// remove first char from r_str
524+
r_str.remove(0);
525+
resource = Some(PathBuf::from(r_str));
526+
}
527+
longest_str = format!("{}/", maybe_revision);
528+
}
529+
530+
log::debug!("Got head_commit: {:?}", head_commit);
531+
log::debug!("Got resource: {:?}", resource);
532+
533+
let head_commit = head_commit.ok_or(OxenError::revision_not_found(head.into()))?;
534+
let resource = resource.ok_or(OxenError::revision_not_found(head.into()))?;
535+
536+
Ok((base_commit, head_commit, resource))
537+
}

src/server/src/routes.rs

+4
Original file line numberDiff line numberDiff line change
@@ -154,6 +154,10 @@ pub fn config(cfg: &mut web::ServiceConfig) {
154154
"/{namespace}/{repo_name}/compare/entries/{base_head:.*}",
155155
web::get().to(controllers::compare::entries),
156156
)
157+
.route(
158+
"/{namespace}/{repo_name}/compare/file/{base_head:.*}",
159+
web::get().to(controllers::compare::file),
160+
)
157161
.route(
158162
"/{namespace}/{repo_name}/compare/data_frame/{compare_id}/{path}/{base_head:.*}",
159163
web::get().to(controllers::compare::get_derived_df),

tests/test_diff.rs

+9-9
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,12 @@
1-
use std::path::Path;
2-
3-
use liboxen::api;
4-
use liboxen::command;
5-
use liboxen::error::OxenError;
6-
use liboxen::model::ContentType;
7-
use liboxen::opts::DFOpts;
8-
use liboxen::test;
9-
use liboxen::util;
1+
// use std::path::Path;
2+
3+
// use liboxen::api;
4+
// use liboxen::command;
5+
// use liboxen::error::OxenError;
6+
// use liboxen::model::ContentType;
7+
// use liboxen::opts::DFOpts;
8+
// use liboxen::test;
9+
// use liboxen::util;
1010

1111
// Test diff during a merge conflict should show conflicts for a dataframe
1212
// #[tokio::test]

0 commit comments

Comments
 (0)