-
Notifications
You must be signed in to change notification settings - Fork 13
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.
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
Push, pull, compact, verify
commands e2e tests
#893
base: chore/e2e-tests
Are you sure you want to change the base?
Conversation
9795293
to
ff8281d
Compare
Push, pull, compact, verify
commands e2e tests
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added minor comments
@@ -152,7 +152,6 @@ impl VerifyCommand { | |||
|
|||
Ok(self | |||
.verification_svc | |||
.clone() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note: A good catch!
|
||
let data = indoc::indoc!( | ||
r#" | ||
{"match_time": "2000-01-01", "match_id": 2, "player_id": "Bob", "score": 90} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
File scope optional comment: use DATASET_ROOT_PLAYER_SCORES_INGEST_DATA_NDJSON_CHUNK_1
after rebase
kamu.ingest_data(&dataset_name, data).await; | ||
let data = indoc::indoc!( | ||
r#" | ||
{"match_time": "2000-01-01", "match_id": 1, "player_id": "Alice", "score": 90} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
File scope optional comment: use DATASET_ROOT_PLAYER_SCORES_INGEST_DATA_NDJSON_CHUNK_2
after rebase
indoc::indoc!( | ||
r#" | ||
let data = indoc::indoc!( | ||
r#" | ||
{"match_time": "2000-01-01", "match_id": 1, "player_id": "Alice", "score": 100} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please replace with a const after rebasing
let kamu_api_server_dataset_endpoint = { | ||
let base_url = kamu_api_server_client.get_base_url(); | ||
|
||
let mut dataset_endpoint = Url::parse("odf+http://host").unwrap(); | ||
|
||
dataset_endpoint.set_host(base_url.host_str()).unwrap(); | ||
dataset_endpoint.set_port(base_url.port()).unwrap(); | ||
|
||
dataset_endpoint | ||
.join(format!("e2e-user/{dataset_name}").as_str()) | ||
.unwrap() | ||
.to_string() | ||
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Q: how about extract in a helper method?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
"--force", | ||
]) | ||
.await | ||
.success(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
global file comment: please validate success message (here and above)
let aliases = kamu_in_push_workspace | ||
.get_list_of_repo_aliases(&opendatafabric::DatasetRef::from(dataset_name.clone())) | ||
.await; | ||
assert_eq!( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
global optional comment: please use pretty_assertions::assert_eq!()
aliases, | ||
vec![RepoAlias { | ||
dataset: dataset_name.clone(), | ||
kind: "Push".to_string(), | ||
alias: kamu_api_server_dataset_endpoint.clone(), | ||
}] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
global optional comment: please use expected & after actual value
let kamu_in_pull_workspace = KamuCliPuppet::new_workspace_tmp().await; | ||
|
||
let new_dataset_name = DatasetName::new_unchecked("foo"); | ||
kamu_in_pull_workspace |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we need to add another step with kamu list
to make sure we save locally the dataset with the new name
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Description
Closes: #issue
Checklist before requesting a review