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.

Sign up for GitHub

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

Jump to bottom

fix: refine patterns test --watch #406

Merged
merged 16 commits into from
Jul 8, 2024
Merged

fix: refine patterns test --watch #406

Show file tree
Hide file tree
Changes from 14 commits
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
c487bb3
[skip ci] yolo
morgante Jul 8, 2024
104f359
[skip ci] yolo
morgante Jul 8, 2024
f311fb8
[skip ci] yolo
morgante Jul 8, 2024
08654e1
[skip ci] yolo
morgante Jul 8, 2024
7a6957a
[skip ci] yolo
morgante Jul 8, 2024
ca86100
[skip ci] yolo
morgante Jul 8, 2024
7d1464b
[skip ci] yolo
morgante Jul 8, 2024
0c232c5
[skip ci] yolo
morgante Jul 8, 2024
d00b01a
[skip ci] yolo
morgante Jul 8, 2024
5965772
run the tests
morgante Jul 8, 2024
8e88a70
[skip ci] yolo
morgante Jul 8, 2024
5dadf0c
run the tests
morgante Jul 8, 2024
8ff14d6
[skip ci] yolo
morgante Jul 8, 2024
93bfc37
run the tests
morgante Jul 8, 2024
ca79f4d
run the tests
morgante Jul 8, 2024
e45355e
run the tests
morgante Jul 8, 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
1 change: 1 addition & 0 deletions Cargo.lock
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

256 changes: 161 additions & 95 deletions crates/cli/src/commands/patterns_test.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
use dashmap::{DashMap, ReadOnlyView};
use log::{debug, info};

use marzano_core::analysis::get_dependents_of_target_patterns_by_traversal_from_src;
use marzano_core::api::MatchResult;
use marzano_gritmodule::config::{GritPatternSample, GritPatternTestInfo};
use marzano_gritmodule::formatting::format_rich_files;
Expand Down Expand Up @@ -30,24 +31,24 @@

use anyhow::{anyhow, bail, Context as _, Result};
use std::collections::HashMap;
use std::path::PathBuf;
use std::{path::Path, time::Duration};

use marzano_core::pattern_compiler::compiler::get_dependents_of_target_patterns_by_traversal_from_src;
use marzano_gritmodule::searcher::collect_from_file;
use notify::{self, RecursiveMode};
use notify_debouncer_mini::{new_debouncer_opt, Config};

pub enum AggregatedTestResult {
SomeFailed(String),
AllPassed,
}

pub async fn get_marzano_pattern_test_results(
patterns: Vec<GritPatternTestInfo>,
libs: &PatternsDirectory,
args: &PatternsTestArgs,
output: OutputFormat,
) -> Result<()> {
if patterns.is_empty() {
bail!("No testable patterns found. To test a pattern, make sure it is defined in .grit/grit.yaml or a .md file in your .grit/patterns directory.");
}
info!("Found {} testable patterns.", patterns.len());

) -> Result<AggregatedTestResult> {
let resolver = GritModuleResolver::new();

let final_results: DashMap<String, Vec<WrappedResult>> = DashMap::new();
Expand Down Expand Up @@ -201,7 +202,7 @@

if args.update {
update_results(&final_results, patterns)?;
return Ok(());
return Ok(AggregatedTestResult::AllPassed);
}

let final_results = final_results.into_read_only();
Expand All @@ -213,15 +214,15 @@
.values()
.any(|v| v.iter().any(|r| !r.result.is_pass()))
{
bail!(
return Ok(AggregatedTestResult::SomeFailed(format!(
"{} out of {} samples failed.",
final_results
.values()
.flatten()
.filter(|r| !r.result.is_pass())
.count(),
total
)
total,
)));
};
info!("✓ All {} samples passed.", total);
}
Expand Down Expand Up @@ -254,7 +255,7 @@
bail!("Output format not supported for this command");
}
}
Ok(())
Ok(AggregatedTestResult::AllPassed)
}

pub(crate) async fn run_patterns_test(
Expand Down Expand Up @@ -284,12 +285,142 @@

let testable_patterns = collect_testable_patterns(patterns);

get_marzano_pattern_test_results(testable_patterns.clone(), &libs, &arg, flags.clone().into())
.await?;
if testable_patterns.is_empty() {
bail!("No testable patterns found. To test a pattern, make sure it is defined in .grit/grit.yaml or a .md file in your .grit/patterns directory.");
}
info!("Found {} testable patterns.", testable_patterns.len());

let first_result = get_marzano_pattern_test_results(
testable_patterns.clone(),
&libs,
&arg,
flags.clone().into(),
)
.await?;

if arg.watch {
if let AggregatedTestResult::SomeFailed(message) = first_result {
println!("{}", message);
}
let _ = enable_watch_mode(testable_patterns, &libs, &arg, flags.into()).await;
Ok(())
} else {
match first_result {
AggregatedTestResult::SomeFailed(message) => bail!(message),
AggregatedTestResult::AllPassed => Ok(()),
}
}
}

fn print_watch_start(path: &Path) {
log::info!(
"\nWatching for changes to {}",
format!("{}", path.display()).bold().underline()
);
}

async fn test_modified_path(
modified_file_path: &PathBuf,

Check failure on line 323 in crates/cli/src/commands/patterns_test.rs

View workflow job for this annotation

GitHub Actions / clippy

writing `&PathBuf` instead of `&Path` involves a new object where a slice will do 

error: writing `&PathBuf` instead of `&Path` involves a new object where a slice will do
   --> crates/cli/src/commands/patterns_test.rs:323:25
    |
323 |     modified_file_path: &PathBuf,
    |                         ^^^^^^^^
    |
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#ptr_arg
    = note: `-D clippy::ptr-arg` implied by `-D warnings`
    = help: to override `-D warnings` add `#[allow(clippy::ptr_arg)]`
help: change this to
    |
323 ~     modified_file_path: &Path,
324 |     testable_patterns: &Vec<GritPatternTestInfo>,
  ...
334 |     }
335 ~     let modified_file_path = modified_file_path.to_path_buf()
    |
testable_patterns: &Vec<GritPatternTestInfo>,
testable_patterns_map: &HashMap<&String, &GritPatternTestInfo>,
libs: &PatternsDirectory,
args: &PatternsTestArgs,
output: OutputFormat,
) -> Result<()> {
let ignore_path = [".grit/.gritmodules", ".grit/.gitignore", ".log"];

if !modified_file_path.is_file() {
return Ok(());
}
let modified_file_path = modified_file_path
.clone()
.into_os_string()
.into_string()
.unwrap();

//temporary fix, until notify crate adds support for ignoring paths
for path in &ignore_path {
if modified_file_path.contains(path) {
return Ok(());
}
}
let (modified_patterns, deleted_patterns) =
get_modified_and_deleted_patterns(&modified_file_path, testable_patterns).await?;

if modified_patterns.is_empty() && deleted_patterns.is_empty() {
log::error!(
"{}",
format!(
"\nFile {} was modified, but no changed patterns were found.",
modified_file_path.bold().underline()
)
.bold()
);
return Ok(());
}

let deleted_patterns_names = deleted_patterns
.iter()
.map(|p| p.local_name.as_ref().unwrap())
.collect::<Vec<_>>();

let mut patterns_to_test = modified_patterns.clone();
let mut patterns_to_test_names = patterns_to_test
.iter()
.map(|p| p.local_name.clone().unwrap())
.collect::<Vec<_>>();

if !modified_patterns.is_empty() {
let modified_patterns_dependents_names =
get_dependents_of_target_patterns(libs, &testable_patterns, &modified_patterns)?;

Check failure on line 375 in crates/cli/src/commands/patterns_test.rs

View workflow job for this annotation

GitHub Actions / clippy

this expression creates a reference which is immediately dereferenced by the compiler 

error: this expression creates a reference which is immediately dereferenced by the compiler
   --> crates/cli/src/commands/patterns_test.rs:375:53
    |
375 |             get_dependents_of_target_patterns(libs, &testable_patterns, &modified_patterns)?;
    |                                                     ^^^^^^^^^^^^^^^^^^ help: change this to: `testable_patterns`
    |
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#needless_borrow
    = note: `-D clippy::needless-borrow` implied by `-D warnings`
    = help: to override `-D warnings` add `#[allow(clippy::needless_borrow)]`
for name in &modified_patterns_dependents_names {
if !deleted_patterns_names.contains(&name) && !patterns_to_test_names.contains(name) {
patterns_to_test.push((*testable_patterns_map.get(name).unwrap()).clone());
patterns_to_test_names.push(name.to_owned());
}
}
}

if !deleted_patterns.is_empty() {
let deleted_patterns_dependents_names =
get_dependents_of_target_patterns(libs, &testable_patterns, &deleted_patterns)?;

Check failure on line 386 in crates/cli/src/commands/patterns_test.rs

View workflow job for this annotation

GitHub Actions / clippy

this expression creates a reference which is immediately dereferenced by the compiler 

error: this expression creates a reference which is immediately dereferenced by the compiler
   --> crates/cli/src/commands/patterns_test.rs:386:53
    |
386 |             get_dependents_of_target_patterns(libs, &testable_patterns, &deleted_patterns)?;
    |                                                     ^^^^^^^^^^^^^^^^^^ help: change this to: `testable_patterns`
    |
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#needless_borrow
for name in &deleted_patterns_dependents_names {
if !deleted_patterns_names.contains(&name) && !patterns_to_test_names.contains(name) {
patterns_to_test.push((*testable_patterns_map.get(name).unwrap()).clone());
patterns_to_test_names.push(name.to_owned());
}
}
}

if patterns_to_test_names.is_empty() {
log::error!(
"{}",
format!(
"\nFile {} was modified, but no testable pattern changes were found.",
modified_file_path.bold().underline()
)
.bold()
);
return Ok(());
}
log::info!(
"{}",
format!(
"\nFile {} was modified, retesting {} patterns:",
modified_file_path.bold().underline(),
patterns_to_test_names.len()
)
.bold()
);

let res =
get_marzano_pattern_test_results(patterns_to_test, libs, args, output.clone()).await?;
match res {
AggregatedTestResult::SomeFailed(message) => {
log::error!("{}", message.to_string().bold().red());
}
AggregatedTestResult::AllPassed => {}
};
Ok(())
}

Expand All @@ -300,7 +431,6 @@
output: OutputFormat,
) -> Result<()> {
let path = Path::new(".grit");
let ignore_path = [".grit/.gritmodules", ".grit/.gitignore", ".log"];
// setup debouncer
let (tx, rx) = std::sync::mpsc::channel();
// notify backend configuration
Expand All @@ -313,7 +443,7 @@
let mut debouncer = new_debouncer_opt::<_, notify::PollWatcher>(debouncer_config, tx)?;

debouncer.watcher().watch(path, RecursiveMode::Recursive)?;
log::info!("\n[Watch Mode] Enabled on path: {}", path.display());
print_watch_start(path);

let testable_patterns_map = testable_patterns
.iter()
Expand All @@ -326,90 +456,26 @@
Ok(event) => {
let modified_file_path = &event.first().unwrap().path;

if !modified_file_path.is_file() {
continue;
}
let modified_file_path = modified_file_path
.clone()
.into_os_string()
.into_string()
.unwrap();

//temporary fix, until notify crate adds support for ignoring paths
for path in &ignore_path {
if modified_file_path.contains(path) {
continue;
}
}
log::info!("\n[Watch Mode] File modified: {:?}", modified_file_path);
let (modified_patterns, deleted_patterns) =
get_modified_and_deleted_patterns(&modified_file_path, &testable_patterns)
.await?;

if modified_patterns.is_empty() && deleted_patterns.is_empty() {
log::info!("[Watch Mode] No patterns changed.\n");
continue;
}

let deleted_patterns_names = deleted_patterns
.iter()
.map(|p| p.local_name.as_ref().unwrap())
.collect::<Vec<_>>();

let mut patterns_to_test = modified_patterns.clone();
let mut patterns_to_test_names = patterns_to_test
.iter()
.map(|p| p.local_name.clone().unwrap())
.collect::<Vec<_>>();

if !modified_patterns.is_empty() {
let modified_patterns_dependents_names = get_dependents_of_target_patterns(
libs,
&testable_patterns,
&modified_patterns,
)?;
for name in &modified_patterns_dependents_names {
if !deleted_patterns_names.contains(&name)
&& !patterns_to_test_names.contains(name)
{
patterns_to_test
.push((*testable_patterns_map.get(name).unwrap()).clone());
patterns_to_test_names.push(name.to_owned());
}
}
}

if !deleted_patterns.is_empty() {
let deleted_patterns_dependents_names = get_dependents_of_target_patterns(
libs,
&testable_patterns,
&deleted_patterns,
)?;
for name in &deleted_patterns_dependents_names {
if !deleted_patterns_names.contains(&name)
&& !patterns_to_test_names.contains(name)
{
patterns_to_test
.push((*testable_patterns_map.get(name).unwrap()).clone());
patterns_to_test_names.push(name.to_owned());
}
let retest = test_modified_path(
modified_file_path,
&testable_patterns,
&testable_patterns_map,
libs,
args,
output.clone(),
)
.await;
match retest {
Ok(_) => {}
Err(error) => {
log::error!("Error: {error:?}")
}
}

log::info!(
"[Watch Mode] Pattern(s) to test: {:?}",
patterns_to_test_names
);
if patterns_to_test_names.is_empty() {
continue;
}

let _ =
get_marzano_pattern_test_results(patterns_to_test, libs, args, output.clone())
.await;
print_watch_start(path);
}
Err(error) => {
log::error!("[Watch Mode] Error: {error:?}")
log::error!("Error: {error:?}")
}
}
}
Expand Down
10 changes: 8 additions & 2 deletions crates/cli/src/commands/plumbing.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -261,7 +261,7 @@ pub(crate) async fn run_plumbing(

let cwd = std::env::current_dir()?;
let libs = get_grit_files_from(Some(cwd)).await?;
get_marzano_pattern_test_results(
let res = get_marzano_pattern_test_results(
patterns,
&libs,
&PatternsTestArgs {
Expand All @@ -273,7 +273,13 @@ pub(crate) async fn run_plumbing(
},
parent.into(),
)
.await
.await?;
match res {
super::patterns_test::AggregatedTestResult::SomeFailed(message) => {
Err(anyhow::anyhow!(message))
}
super::patterns_test::AggregatedTestResult::AllPassed => Ok(()),
}
}
}
}
2 changes: 0 additions & 2 deletions crates/cli_bin/fixtures/.grit/.gitignore
View file
Open in desktop

This file was deleted.

Loading
Loading