Skip to content
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

Open
wants to merge 89 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 85 commits
Commits
Show all changes
89 commits
Select commit Hold shift + click to select a range
3c4a076
Add expandable shell snippet outputs
franciszekjob Nov 8, 2024
f8b0058
Change shell snippets to expandable blocks
franciszekjob Nov 9, 2024
4e45711
Make expandable snippet open by default
franciszekjob Nov 10, 2024
23493ab
Make `tree` commands expandable
franciszekjob Nov 10, 2024
62bb35a
Restore prompt in shell snippets
franciszekjob Nov 10, 2024
3ef847f
Revert "Restore prompt in shell snippets"
franciszekjob Nov 10, 2024
304e423
Restore prompt in shell snippets
franciszekjob Nov 10, 2024
f10ac8e
Restore prompt in other shell snippets
franciszekjob Nov 10, 2024
be9c9e5
Fix forge version check snippet
franciszekjob Nov 10, 2024
a17d919
Add JS script to remove prompt sign when copying code
franciszekjob Nov 10, 2024
432e366
Add validation of sncast snippets
franciszekjob Nov 11, 2024
2cc325c
Merge branch 'franciszekjob/2479-expandable-shell-snippets' of https:…
franciszekjob Nov 11, 2024
f5f3e41
Remove unnecessary else branch
franciszekjob Nov 12, 2024
beb0855
Fix left docs snippets
franciszekjob Nov 12, 2024
18ffce2
Make `tree` snippet expandable in first steps section
franciszekjob Nov 12, 2024
1d7f284
Fix typo
franciszekjob Nov 12, 2024
48a3fcd
Fix typo
franciszekjob Nov 12, 2024
0438352
Fix typos
franciszekjob Nov 12, 2024
ccfac3f
Merge branch 'master' into franciszekjob/2479-expandable-shell-snippets
franciszekjob Nov 12, 2024
dca617d
Improve sncast snippets validation test
franciszekjob Nov 12, 2024
ed6daa0
Merge branch 'franciszekjob/2479-expandable-shell-snippets' of https:…
franciszekjob Nov 12, 2024
3ae3107
Fix printing number of validated snippets in sncast test
franciszekjob Nov 12, 2024
2f65010
Add snforge docs snippets validation test
franciszekjob Nov 12, 2024
efc2e1d
Remove print
franciszekjob Nov 12, 2024
2823523
Merge branch 'master' into franciszekjob/2479-expandable-shell-snippets
franciszekjob Nov 12, 2024
bcbb861
Merge branch 'franciszekjob/2479-expandable-shell-snippets' into fran…
franciszekjob Nov 12, 2024
1b968be
Move docs validation into separate crate
franciszekjob Nov 12, 2024
041f604
Merge branch 'franciszekjob/2479-2-shell-commands-validation' of http…
franciszekjob Nov 12, 2024
7bbcd99
Fix newline
franciszekjob Nov 12, 2024
bdcd467
Update `Cargo.toml` in `sncast`
franciszekjob Nov 12, 2024
b6db6a5
Test cast docs validation test
franciszekjob Nov 12, 2024
a4535cd
Fix linting
franciszekjob Nov 12, 2024
dcd4e80
Cleanup sncast snippets validation test
franciszekjob Nov 13, 2024
9e34797
Minor refactor of forge snippets validation test
franciszekjob Nov 13, 2024
8ddc850
Fix linting
franciszekjob Nov 13, 2024
8e6678c
Update `Cargo.lock`
franciszekjob Nov 13, 2024
6da2d58
Merge branch 'master' of https://github.com/foundry-rs/starknet-found…
franciszekjob Nov 14, 2024
92fff9f
Fix deploy contract snippet
franciszekjob Nov 14, 2024
d445531
Use `try_parse_from` in forge docs snippets validation
franciszekjob Nov 15, 2024
46aa9d4
Add skipped args in forge
franciszekjob Nov 15, 2024
1dd0062
Refactor snippets validation test in cast
franciszekjob Nov 15, 2024
80e5b79
Add todo
franciszekjob Nov 15, 2024
54f4247
Code cleanup
franciszekjob Nov 15, 2024
fab960d
Fix typos
franciszekjob Nov 15, 2024
f119e3b
Minor fixes in docs
franciszekjob Nov 18, 2024
853c8ed
Remove `license-file.workspace` from docs crate
franciszekjob Nov 18, 2024
f6a029c
Move todo
franciszekjob Nov 18, 2024
2e0f0f1
Merge branch 'master' into franciszekjob/2479-2-shell-commands-valida…
franciszekjob Nov 18, 2024
41cb86a
Show validated snippets number
franciszekjob Nov 18, 2024
6d8a825
Merge branch 'franciszekjob/2479-2-shell-commands-validation' of http…
franciszekjob Nov 18, 2024
5c84ef1
Fix linting
franciszekjob Nov 18, 2024
8292a54
Display line and file path of failed snippet
franciszekjob Nov 18, 2024
fa40a6b
Fix err message
franciszekjob Nov 18, 2024
d846b42
Code review improvements
franciszekjob Nov 18, 2024
cbda065
Move docs to dev dependencies
franciszekjob Nov 18, 2024
9fe0a3c
Update `tree` output in first steps sections
franciszekjob Nov 19, 2024
906777b
Allow setup packages from docs listings
franciszekjob Nov 19, 2024
446cdfd
Add example packages for "Running tests" section
franciszekjob Nov 19, 2024
2a61d99
wip: Update outputs for snippets in snforge overview
franciszekjob Nov 19, 2024
bcf5f40
Fix typo
franciszekjob Nov 19, 2024
aa4be23
wip: Update outputs for cheatcode snippets
franciszekjob Nov 19, 2024
8449f5e
Update outputs for workspaces
franciszekjob Nov 19, 2024
ece5a07
Merge branch 'master' of https://github.com/foundry-rs/starknet-found…
franciszekjob Nov 19, 2024
a960649
Update command outputs docs
franciszekjob Nov 20, 2024
826be45
Update packages and manifests in docs listings
franciszekjob Nov 20, 2024
0574214
Add `SnippetType` enum; Add `output` and `snippet_type` fields in `Sn…
franciszekjob Nov 20, 2024
67346ea
Add `setup_package_from_docs_listings`
franciszekjob Nov 20, 2024
127842a
Add util functions to retrieve mapping of docs listing -> crates
franciszekjob Nov 20, 2024
f01065d
Add command execution in forge docs snippets test
franciszekjob Nov 20, 2024
6caf67b
Fix linting
franciszekjob Nov 20, 2024
eb95205
Run `scarb fmt`
franciszekjob Nov 20, 2024
1c2a863
Use `scarb check` in `verify_cairo_listings.sh`
franciszekjob Nov 20, 2024
95d9bb9
Fix linting
franciszekjob Nov 20, 2024
d8481b0
Run `scarb fmt`
franciszekjob Nov 20, 2024
09bac46
Fix typo
franciszekjob Nov 20, 2024
ae39133
Remove comment
franciszekjob Nov 21, 2024
7703ce6
Update sncast docs snippets test
franciszekjob Nov 21, 2024
a106627
Refactor `test_docs_snippets` in forge
franciszekjob Nov 21, 2024
4407733
Refactor util functions
franciszekjob Nov 21, 2024
983e0a6
Change err msg while calling `extract_snippets_from_directory` in forge
franciszekjob Nov 21, 2024
78e3c7d
Format
franciszekjob Nov 21, 2024
97df99f
Add todo
franciszekjob Nov 21, 2024
dc4386c
Remove unused package
franciszekjob Nov 21, 2024
4d245af
Allow ignoring snippets
franciszekjob Nov 21, 2024
0eadab4
Fix failing tests
franciszekjob Nov 21, 2024
c5e3b19
Apply CR suggestions
franciszekjob Nov 21, 2024
4df85ac
Add `hello_workspaces` case in `resolve_temp_dir`
franciszekjob Nov 21, 2024
216c10b
Run `scarb clean` before `scarb check` in `verify_cairo_listings.sh`
franciszekjob Nov 21, 2024
a2f01d5
Revert "Run `scarb clean` before `scarb check` in `verify_cairo_listi…
franciszekjob Nov 21, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
134 changes: 119 additions & 15 deletions crates/docs/src/validation.rs
Original file line number Diff line number Diff line change
@@ -1,15 +1,47 @@
use regex::Regex;
use std::{
collections::HashMap,
env, fs, io,
path::{Path, PathBuf},
};
use walkdir::WalkDir;

const EXTENSION: Option<&str> = Some("md");

#[derive(Clone, Debug)]
pub enum SnippetType {
franciszekjob marked this conversation as resolved.
Show resolved Hide resolved
Forge,
Sncast,
}

impl SnippetType {
#[must_use]
pub fn as_str(&self) -> &str {
match self {
SnippetType::Forge => "snforge",
SnippetType::Sncast => "sncast",
}
}

#[must_use]
pub fn get_re(&self) -> Regex {
let pattern = format!(
r"(?ms)^(?:<!--\s*(.*?)\s*-->\n)?```shell\n\$ ({} .+?)\n```(?:\s*<details>\n<summary>Output:<\/summary>\n\n```shell\n([\s\S]+?)\n```[\s]*<\/details>)?",
self.as_str()
);

Regex::new(&pattern).unwrap()
}
}

#[derive(Debug)]
pub struct Snippet {
pub command: String,
pub output: Option<String>,
pub file_path: String,
pub line_start: usize,
pub snippet_type: SnippetType,
pub ignored: bool,
}

impl Snippet {
Expand All @@ -28,33 +60,56 @@ impl Snippet {
.map(|arg| arg.trim().to_string())
.collect()
}

#[must_use]
pub fn capture_package_from_output(&self) -> Option<String> {
let re =
Regex::new(r"Collected \d+ test\(s\) from ([a-zA-Z_][a-zA-Z0-9_]*) package").unwrap();

re.captures_iter(self.output.as_ref()?)
.filter_map(|caps| caps.get(1))
.last()
.map(|m| m.as_str().to_string())
}
}

pub fn extract_snippets_from_file(file_path: &Path, re: &Regex) -> io::Result<Vec<Snippet>> {
pub fn extract_snippets_from_file(
file_path: &Path,
snippet_type: &SnippetType,
) -> io::Result<Vec<Snippet>> {
let content = fs::read_to_string(file_path)?;
let file_path_str = file_path
.to_str()
.expect("Failed to get file path")
.to_string();

let snippets = re
let snippets = snippet_type
.get_re()
.captures_iter(&content)
.filter_map(|caps| {
let command_match = caps.get(1)?;
let command_match = caps.get(2)?;
let match_start = caps.get(0)?.start();
let output = caps.get(3).map(|m| m.as_str().to_string());
let ignored = caps.get(1).map_or(false, |m| m.as_str().contains("ignore"));

Some(Snippet {
command: command_match.as_str().to_string(),
output,
file_path: file_path_str.clone(),
line_start: content[..match_start].lines().count() + 1,
snippet_type: snippet_type.clone(),
ignored,
})
})
.collect();

Ok(snippets)
}

pub fn extract_snippets_from_directory(dir_path: &Path, re: &Regex) -> io::Result<Vec<Snippet>> {
pub fn extract_snippets_from_directory(
dir_path: &Path,
snippet_type: &SnippetType,
) -> io::Result<Vec<Snippet>> {
let mut all_snippets = Vec::new();

let files = walkdir::WalkDir::new(dir_path)
Expand All @@ -68,7 +123,7 @@ pub fn extract_snippets_from_directory(dir_path: &Path, re: &Regex) -> io::Resul
if EXTENSION.map_or(true, |ext| {
path.extension().and_then(|path_ext| path_ext.to_str()) == Some(ext)
}) {
let snippets = extract_snippets_from_file(path, re)?;
let snippets = extract_snippets_from_file(path, snippet_type)?;
all_snippets.extend(snippets);
}
}
Expand All @@ -90,26 +145,75 @@ pub fn get_parent_dir(levels_up: usize) -> PathBuf {
dir
}

pub fn assert_valid_snippet(
condition: bool,
snippet: &Snippet,
tool_name: &str,
err_message: &str,
) {
pub fn assert_valid_snippet(condition: bool, snippet: &Snippet, err_message: &str) {
assert!(
condition,
"Found invalid {} snippet in the docs in file: {} at line {}\n{}",
tool_name, snippet.file_path, snippet.line_start, err_message
"Found invalid {} snippet in the docs in at {}:{}:1\n{}",
snippet.snippet_type.as_str(),
snippet.file_path,
snippet.line_start,
err_message
);
}

pub fn print_success_message(snippets_len: usize, tool_name: &str) {
println!("Successfully validated {snippets_len} {tool_name} docs snippets");
}

pub fn print_skipped_snippet_message(snippet: &Snippet, tool_name: &str) {
pub fn print_skipped_snippet_message(snippet: &Snippet) {
println!(
"Skipped validation of {} snippet in the docs in file: {} at line {}",
tool_name, snippet.file_path, snippet.line_start
snippet.snippet_type.as_str(),
snippet.file_path,
snippet.line_start,
);
}

pub fn create_listings_to_packages_mapping() -> HashMap<String, Vec<String>> {
let docs_listings_path = "../../docs/listings";
let mut mapping = HashMap::new();

for listing in WalkDir::new(docs_listings_path)
.min_depth(1)
.max_depth(1)
.into_iter()
.filter_map(Result::ok)
.filter(|e| e.file_type().is_dir())
{
let listing_path = listing.path();
let crates_dir = listing_path.join("crates");

if crates_dir.is_dir() {
let packages = list_packages_in_directory(&crates_dir);
if let Some(listing_name) = listing_path.file_name().and_then(|x| x.to_str()) {
if !packages.is_empty() {
mapping.insert(listing_name.to_string(), packages);
}
}
}
}

mapping
}
Comment on lines +176 to +201
Copy link
Member

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?


fn list_packages_in_directory(dir_path: &Path) -> Vec<String> {
let crates_path = dir_path.join("");
franciszekjob marked this conversation as resolved.
Show resolved Hide resolved

if crates_path.exists() && crates_path.is_dir() {
WalkDir::new(crates_path)
.min_depth(1)
.max_depth(1)
.into_iter()
.filter_map(Result::ok)
.filter(|e| e.path().is_dir())
.filter_map(|e| {
e.path()
.file_name()
.and_then(|name| name.to_str())
.map(String::from)
})
.collect()
} else {
Vec::new()
}
}
42 changes: 38 additions & 4 deletions crates/forge/tests/e2e/common/runner.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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()
Expand All @@ -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
Copy link
Member

Choose a reason for hiding this comment

The 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 setup_package_with_file_patterns as well.

Copy link
Member

Choose a reason for hiding this comment

The 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
Expand All @@ -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) {
Expand Down
84 changes: 66 additions & 18 deletions crates/forge/tests/e2e/docs_snippets_validation.rs
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"].contains(&package_name) {
setup_hello_workspace()
} else {
setup_package(package_name)
}
}
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

note: Currently we use hello_workpaces package (from crates/forge/tests/data) in one example in docs. In future we can simplify this logic and remove is_package_from_docs_listings, once we use only packages from docs listings. Ref: #2698

Loading
Loading