-
Notifications
You must be signed in to change notification settings - Fork 177
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
Validate forge command outputs in docs #2687
base: master
Are you sure you want to change the base?
Changes from all commits
3c4a076
f8b0058
4e45711
23493ab
62bb35a
3ef847f
304e423
f10ac8e
be9c9e5
a17d919
432e366
2cc325c
f5f3e41
beb0855
18ffce2
1d7f284
48a3fcd
0438352
ccfac3f
dca617d
ed6daa0
3ae3107
2f65010
efc2e1d
2823523
bcbb861
1b968be
041f604
7bbcd99
bdcd467
b6db6a5
a4535cd
dcd4e80
9e34797
8ddc850
8e6678c
6da2d58
92fff9f
d445531
46aa9d4
1dd0062
80e5b79
54f4247
fab960d
f119e3b
853c8ed
f6a029c
2e0f0f1
41cb86a
6d8a825
5c84ef1
8292a54
fa40a6b
d846b42
cbda065
9fe0a3c
906777b
446cdfd
2a61d99
bcf5f40
aa4be23
8449f5e
ece5a07
a960649
826be45
0574214
67346ea
127842a
f01065d
6caf67b
eb95205
1c2a863
95d9bb9
d8481b0
09bac46
ae39133
7703ce6
a106627
4407733
983e0a6
78e3c7d
97df99f
dc4386c
4d245af
0eadab4
c5e3b19
4df85ac
216c10b
a2f01d5
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -5,6 +5,7 @@ use indoc::formatdoc; | |
use shared::command::CommandExt; | ||
use shared::test_utils::node_url::node_rpc_url; | ||
use snapbox::cmd::{cargo_bin, Command as SnapboxCommand}; | ||
use std::collections::HashMap; | ||
use std::path::{Path, PathBuf}; | ||
use std::process::Command; | ||
use std::str::FromStr; | ||
|
@@ -34,12 +35,17 @@ pub(crate) fn test_runner(temp_dir: &TempDir) -> SnapboxCommand { | |
pub(crate) static BASE_FILE_PATTERNS: &[&str] = &["**/*.cairo", "**/*.toml"]; | ||
|
||
pub(crate) fn setup_package_with_file_patterns( | ||
package_name: &str, | ||
package_path: &str, | ||
file_patterns: &[&str], | ||
) -> TempDir { | ||
let temp = tempdir_with_tool_versions().unwrap(); | ||
temp.copy_from(format!("tests/data/{package_name}"), file_patterns) | ||
.unwrap(); | ||
let package_path = Utf8PathBuf::from_str(package_path) | ||
.unwrap() | ||
.canonicalize_utf8() | ||
.unwrap() | ||
.to_string() | ||
.replace('\\', "/"); | ||
temp.copy_from(package_path.clone(), file_patterns).unwrap(); | ||
|
||
let snforge_std_path = Utf8PathBuf::from_str("../../snforge_std") | ||
.unwrap() | ||
|
@@ -60,6 +66,12 @@ pub(crate) fn setup_package_with_file_patterns( | |
value(get_assert_macros_version().unwrap().to_string()); | ||
scarb_toml["target.starknet-contract"]["sierra"] = value(true); | ||
|
||
if package_path.contains("docs/listings") { | ||
scarb_toml["dev-dependencies"]["snforge_std"] | ||
.as_table_mut() | ||
.and_then(|snforge_std| snforge_std.remove("workspace")); | ||
} | ||
Comment on lines
+69
to
+73
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Don't think we should be hardcoding some docs specific behaviors here. This could be a separate function that shares the logic with this one. + I think you wouldn't have to change the interface of current There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I see that you created one below, but I'd try to change this so the interface of tests we already had remains unchanged. |
||
|
||
manifest_path.write_str(&scarb_toml.to_string()).unwrap(); | ||
|
||
// TODO (#2074): do that on .cairo.template files only | ||
|
@@ -69,7 +81,29 @@ pub(crate) fn setup_package_with_file_patterns( | |
} | ||
|
||
pub(crate) fn setup_package(package_name: &str) -> TempDir { | ||
setup_package_with_file_patterns(package_name, BASE_FILE_PATTERNS) | ||
let package_path = "tests/data/".to_string() + package_name; | ||
setup_package_with_file_patterns(&package_path, BASE_FILE_PATTERNS) | ||
} | ||
|
||
fn get_listing_name( | ||
package: &str, | ||
packages_mapping: &HashMap<String, Vec<String>>, | ||
) -> Option<String> { | ||
packages_mapping | ||
.iter() | ||
.find(|(_, packages)| packages.contains(&package.to_string())) | ||
.map(|(listing_name, _)| listing_name.clone()) | ||
} | ||
|
||
pub(crate) fn setup_package_from_docs_listings( | ||
package_name: &str, | ||
packages_mapping: &HashMap<String, Vec<String>>, | ||
) -> TempDir { | ||
let listing_name = get_listing_name(package_name, packages_mapping) | ||
.unwrap_or_else(|| panic!("Couldn't find listing for package {package_name}")); | ||
let package_path = format!("../../docs/listings/{listing_name}/crates/{package_name}",); | ||
|
||
setup_package_with_file_patterns(&package_path, BASE_FILE_PATTERNS) | ||
} | ||
|
||
fn replace_node_rpc_url_placeholders(dir_path: &Path) { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,43 +1,91 @@ | ||
use std::collections::HashMap; | ||
|
||
use assert_fs::TempDir; | ||
use clap::Parser; | ||
use docs::validation::{ | ||
assert_valid_snippet, extract_snippets_from_directory, get_parent_dir, | ||
print_skipped_snippet_message, print_success_message, | ||
assert_valid_snippet, create_listings_to_packages_mapping, extract_snippets_from_directory, | ||
get_parent_dir, print_skipped_snippet_message, print_success_message, SnippetType, | ||
}; | ||
use forge::Cli; | ||
use regex::Regex; | ||
use shared::test_utils::output_assert::assert_stdout_contains; | ||
|
||
use super::common::runner::{ | ||
setup_hello_workspace, setup_package, setup_package_from_docs_listings, test_runner, | ||
}; | ||
|
||
fn is_package_from_docs_listings( | ||
package: &str, | ||
listings_to_packages_mapping: &HashMap<String, Vec<String>>, | ||
) -> bool { | ||
for packages in listings_to_packages_mapping.values() { | ||
if packages.contains(&package.to_string()) { | ||
return true; | ||
} | ||
} | ||
false | ||
} | ||
|
||
#[test] | ||
fn test_docs_snippets() { | ||
let listings_to_packages_mapping = create_listings_to_packages_mapping(); | ||
|
||
let root_dir = get_parent_dir(2); | ||
let docs_dir = root_dir.join("docs/src"); | ||
|
||
let re = Regex::new(r"(?ms)```shell\n\$ (snforge .+?)\n```").expect("Invalid regex pattern"); | ||
|
||
let snippets = extract_snippets_from_directory(&docs_dir, &re) | ||
.expect("Failed to extract snforge command snippets"); | ||
let snippet_type = SnippetType::forge(); | ||
|
||
// TODO(#2684) | ||
let skipped_args = [ | ||
// for some reason `try_parse_from` fails on `--version` flag | ||
vec!["snforge", "--version"], | ||
]; | ||
let snippets = extract_snippets_from_directory(&docs_dir, &snippet_type) | ||
.expect("Failed to extract command snippets"); | ||
|
||
for snippet in &snippets { | ||
let args = snippet.to_command_args(); | ||
let args: Vec<&str> = args.iter().map(String::as_str).collect(); | ||
let mut args: Vec<&str> = args.iter().map(String::as_str).collect(); | ||
|
||
if skipped_args.contains(&args) { | ||
print_skipped_snippet_message(snippet, "snforge"); | ||
if snippet.ignored { | ||
print_skipped_snippet_message(snippet); | ||
continue; | ||
} | ||
|
||
let parse_result = Cli::try_parse_from(args); | ||
let parse_result = Cli::try_parse_from(args.clone()); | ||
let err_message = if let Err(err) = &parse_result { | ||
err.to_string() | ||
} else { | ||
String::new() | ||
}; | ||
assert_valid_snippet(parse_result.is_ok(), snippet, "snforge", &err_message); | ||
|
||
assert_valid_snippet(parse_result.is_ok(), snippet, &err_message); | ||
|
||
// Remove "snforge" from the args | ||
args.remove(0); | ||
|
||
// Remove "test" from the args | ||
args.retain(|element| element != &"test"); | ||
|
||
if let Some(snippet_output) = &snippet.output { | ||
let package_name = snippet | ||
.capture_package_from_output() | ||
.expect("Failed to capture package from command output"); | ||
|
||
// TODO(#2698) | ||
let temp = resolve_temp_dir(&package_name, &listings_to_packages_mapping); | ||
let output = test_runner(&temp).args(args).assert(); | ||
|
||
assert_stdout_contains(output, snippet_output); | ||
} | ||
} | ||
|
||
print_success_message(snippets.len(), "snforge"); | ||
print_success_message(snippets.len(), snippet_type.as_str()); | ||
} | ||
|
||
fn resolve_temp_dir( | ||
package_name: &str, | ||
listings_to_packages_mapping: &HashMap<String, Vec<String>>, | ||
) -> TempDir { | ||
if is_package_from_docs_listings(package_name, listings_to_packages_mapping) { | ||
setup_package_from_docs_listings(package_name, listings_to_packages_mapping) | ||
} else if ["addition", "fibonacci", "hello_workspaces"].contains(&package_name) { | ||
setup_hello_workspace() | ||
} else { | ||
setup_package(package_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.
Isn't this logic a bit too complicated just to get a mapping between a directory name and it's related crates?