diff --git a/.config/nextest.toml b/.config/nextest.toml index 715941a..4beb89f 100644 --- a/.config/nextest.toml +++ b/.config/nextest.toml @@ -10,12 +10,16 @@ success-output = "never" # fixtures and we want to see them all in one run. fail-fast = false -# Network-gated real-world roundtrip. Absorb flaky DNS/HTTP with a -# couple of retries, warn loudly when a single attempt takes more than +# Network-gated real-world roundtrip overrides. Absorb flaky DNS/HTTP +# with a couple of retries, warn when a single attempt takes more than # 30s (a dead resolver, usually), and hard-kill after 3 minutes to -# prevent wedged CI. Deterministic tests still fail fast via the -# default profile's settings. +# prevent wedged CI. The binary filter covers every `roundtrip_*` test +# added via the `roundtrip_test!` macro. +# +# No test-group here: nextest runs each test in its own OS process, so +# `std::env::set_current_dir` calls are isolated per test. Each of the +# 12 tests targets a different authority, so parallel DNS/HTTP is fine. [[profile.default.overrides]] -filter = "test(test_real_world_roundtrip)" +filter = "binary(real_world_roundtrip)" retries = 2 slow-timeout = { period = "30s", terminate-after = 6 } diff --git a/justfile b/justfile index a9f5d62..3feafcb 100644 --- a/justfile +++ b/justfile @@ -53,12 +53,10 @@ test-workspace: @echo "\nRunning workspace resolution tests..." cargo nextest run -p mlf-integration-tests --test workspace_integration -# Run real-world round-trip tests (network-dependent, ignored by default) +# Run only the network-dependent real-world round-trip tests test-real-world: @echo "\n🌐 Running real-world round-trip tests (fetches from network)..." - @echo "This will download lexicons from: app.bsky.*, net.anisota.*, place.stream.*, pub.leaflet.*" - @echo "" - cargo nextest run -p mlf-integration-tests --test real_world_roundtrip --run-ignored only + cargo nextest run -p mlf-integration-tests --test real_world_roundtrip # Run tests with verbose output (stdout streamed live, tests run serially) test-verbose: diff --git a/tests/real_world/roundtrip.rs b/tests/real_world/roundtrip.rs index af8b54e..5401aca 100644 --- a/tests/real_world/roundtrip.rs +++ b/tests/real_world/roundtrip.rs @@ -1,108 +1,128 @@ -// Real-world round-trip tests: JSON → MLF → JSON +// Real-world round-trip tests: JSON → MLF → JSON. // -// These tests fetch real lexicons from the network, convert them to MLF, -// then generate JSON back and verify the round-trip is accurate. +// Each test fetches real lexicons from the network for a single authority +// pattern (e.g. `app.bsky.actor.*`), converts them to MLF via the fetcher, +// regenerates JSON from the MLF using the codegen library, and verifies +// the round-trip is semantically accurate. // -// Run with: cargo test --test real_world_roundtrip -- --ignored --nocapture +// These tests hit the live network. Target one authority with: +// +// cargo nextest run --test real_world_roundtrip roundtrip_app_bsky_actor +// +// Each test chdir's into its own tempdir before invoking the CLI library +// API (which still reads the project root from the process CWD). That's +// safe under nextest because nextest runs each test in its own OS +// process, so CWD changes are isolated — the 12 roundtrips parallelise +// freely. use std::collections::HashSet; use std::fs; use std::path::{Path, PathBuf}; use std::process::Command; + +use mlf_cli::fetch::run_fetch; +use mlf_cli::workspace_ext::load_mlf_directory; +use mlf_codegen::generate_lexicon; +use mlf_lang::Workspace; use tempfile::TempDir; -/// Real-world lexicon sources to test -/// These use specific namespaces that have DNS TXT records published -const TEST_SOURCES: &[&str] = &[ - // Bluesky - use specific namespaces since top-level doesn't have TXT record - "app.bsky.actor.*", - "app.bsky.feed.*", - "app.bsky.graph.*", - // Other networks - "net.anisota.*", - "place.stream.*", - "pub.leaflet.*", -]; - -#[test] -#[ignore] // Network-dependent test, run explicitly with --ignored -fn test_real_world_roundtrip() { - println!("\n🌐 Real-World Round-Trip Test"); - println!("=============================\n"); - - // Create temp directory for test workspace +/// Fetch the given authority pattern, round-trip every lexicon through +/// MLF, and assert nothing structurally changed along the way. +fn run_source_roundtrip(source: &str) { + println!("\n🌐 Round-trip for {}\n", source); + let temp_dir = TempDir::new().expect("Failed to create temp directory"); let workspace_path = temp_dir.path(); - println!("šŸ“ Test workspace: {}\n", workspace_path.display()); - - // Step 1: Initialize MLF project - println!("1ļøāƒ£ Initializing MLF project..."); init_mlf_project(workspace_path).expect("Failed to initialize project"); - // Step 2: Fetch real lexicons - println!("\n2ļøāƒ£ Fetching real lexicons from network..."); - for source in TEST_SOURCES { - println!(" Fetching: {}", source); - fetch_lexicons(workspace_path, source).expect(&format!("Failed to fetch {}", source)); - } - - // Step 3: Copy MLF files to standard lexicons directory - println!("\n3ļøāƒ£ Copying MLF files to standard lexicons directory..."); - let source_mlf_dir = workspace_path.join(".mlf/lexicons/mlf"); - let lexicons_dir = workspace_path.join("lexicons"); - copy_mlf_files(&source_mlf_dir, &lexicons_dir).expect("Failed to copy MLF files"); - - // Step 4: Generate JSON from MLF - println!("\n4ļøāƒ£ Generating JSON from MLF files..."); - let output_dir = workspace_path.join("generated-lexicons"); - generate_json_from_mlf(workspace_path, &output_dir).expect("Failed to generate JSON"); - - // Step 5: Compare original vs regenerated JSON - println!("\n5ļøāƒ£ Comparing original vs regenerated JSON..."); - let original_dir = workspace_path.join(".mlf/lexicons/json"); - - // Write diffs to tests/real_world/roundtrip/diffs/ (persisted, gitignored) - let diffs_dir = std::path::PathBuf::from(env!("CARGO_MANIFEST_DIR")) - .join("real_world/roundtrip/diffs"); - - let stats = compare_json_files(&original_dir, &output_dir, &diffs_dir) - .expect("Failed to compare JSON files"); - - // Step 6: Report results - println!("\nšŸ“Š Round-Trip Test Results"); - println!("==========================="); - println!("Total lexicons tested: {}", stats.total); - println!("Perfect matches: {}", stats.perfect_matches); - println!("Acceptable differences: {}", stats.acceptable_diffs); - println!("Failures: {}", stats.failures); + // The CLI's fetch + generate APIs read the project root from the + // process's current working directory. Nextest runs each test in its + // own OS process, so this chdir is isolated — a sibling roundtrip + // running in parallel sees its own CWD. + std::env::set_current_dir(workspace_path) + .unwrap_or_else(|e| panic!("Failed to chdir to {}: {}", workspace_path.display(), e)); + + let runtime = tokio::runtime::Builder::new_current_thread() + .enable_all() + .build() + .expect("Failed to build tokio runtime"); + runtime + .block_on(run_fetch(Some(source.to_string()), false, false, false)) + .unwrap_or_else(|e| panic!("Failed to fetch {}: {:?}", source, e)); + + let original_json_dir = workspace_path.join(".mlf/lexicons/json"); + let fetched_mlf_dir = workspace_path.join(".mlf/lexicons/mlf"); + + let regenerated = regenerate_lexicons_from_mlf(&fetched_mlf_dir) + .unwrap_or_else(|e| panic!("Failed to regenerate JSON: {}", e)); + + let diffs_dir = PathBuf::from(env!("CARGO_MANIFEST_DIR")) + .join("real_world/roundtrip/diffs") + .join(sanitize_source_name(source)); + + let stats = compare_regenerated(&original_json_dir, ®enerated, &diffs_dir) + .unwrap_or_else(|e| panic!("Comparison failed: {}", e)); + + println!( + "\nšŸ“Š {}: {} total, {} perfect, {} acceptable, {} failures", + source, stats.total, stats.perfect_matches, stats.acceptable_diffs, stats.failures + ); if !stats.failed_lexicons.is_empty() { - println!("\nāŒ Failed lexicons:"); + println!("āŒ Failed lexicons:"); for (nsid, reason) in &stats.failed_lexicons { println!(" - {}: {}", nsid, reason); } } if stats.acceptable_diffs > 0 || stats.failures > 0 { - println!("\nšŸ“ Diff files written to: {}", diffs_dir.display()); - println!(" Review these files to see what changed between original and regenerated JSON"); + println!("šŸ“ Diffs: {}", diffs_dir.display()); } - // Assert that we have no failures assert_eq!( stats.failures, 0, - "Round-trip test failed for {} lexicon(s). Check diff files in {}", + "Round-trip failed for {} lexicon(s) under {}. See {}", stats.failures, + source, diffs_dir.display() ); +} + +fn sanitize_source_name(source: &str) -> String { + source.replace('.', "_").replace('*', "wildcard") +} - println!("\nāœ… All round-trip tests passed!"); +/// Declare one `#[test]` per authority pattern so nextest can report +/// failures per authority. Execution is rate-limited by the +/// `real-world-network` test group in `.config/nextest.toml`. +macro_rules! roundtrip_test { + ($name:ident, $source:expr) => { + #[test] + fn $name() { + run_source_roundtrip($source); + } + }; } -/// Initialize an MLF project with mlf.toml +// Bluesky — the top-level NSID has no TXT record, so we hit each category +// individually. +roundtrip_test!(roundtrip_app_bsky_actor, "app.bsky.actor.*"); +roundtrip_test!(roundtrip_app_bsky_feed, "app.bsky.feed.*"); +roundtrip_test!(roundtrip_app_bsky_graph, "app.bsky.graph.*"); + +// Other published lexicon authorities. +roundtrip_test!(roundtrip_net_anisota, "net.anisota.*"); +roundtrip_test!(roundtrip_place_stream, "place.stream.*"); +roundtrip_test!(roundtrip_pub_leaflet, "pub.leaflet.*"); +roundtrip_test!(roundtrip_social_grain, "social.grain.*"); +roundtrip_test!(roundtrip_at_margin, "at.margin.*"); +roundtrip_test!(roundtrip_blog_pckt, "blog.pckt.*"); + +/// Write a minimal mlf.toml to the workspace root. `run_fetch` searches +/// upward for this file to anchor the project; without it the fetch step +/// would prompt interactively. fn init_mlf_project(workspace_path: &Path) -> Result<(), String> { - // Create mlf.toml let mlf_toml = r#" [package] name = "roundtrip-test" @@ -120,115 +140,63 @@ optimize_transitive_fetches = false Ok(()) } -/// Fetch lexicons using `mlf fetch` -fn fetch_lexicons(workspace_path: &Path, nsid_pattern: &str) -> Result<(), String> { - let output = Command::new("mlf") - .arg("fetch") - .arg(nsid_pattern) - .current_dir(workspace_path) - .output() - .map_err(|e| format!("Failed to execute mlf fetch: {}", e))?; - - if !output.status.success() { - return Err(format!( - "mlf fetch failed:\n{}", - String::from_utf8_lossy(&output.stderr) - )); - } - - Ok(()) -} - -/// Copy MLF files from .mlf/lexicons/mlf to lexicons/ -fn copy_mlf_files(source_dir: &Path, dest_dir: &Path) -> Result<(), String> { - if !source_dir.exists() { - return Err(format!("Source directory not found: {}", source_dir.display())); - } +/// Load every `.mlf` file fetched under the given directory into a +/// workspace, resolve it, and regenerate Lexicon JSON for each module. +/// Returns a map keyed by the on-disk path relative to `mlf_dir` (so +/// callers can line up against the original JSON tree). +fn regenerate_lexicons_from_mlf(mlf_dir: &Path) -> Result, String> { + let mut ws = Workspace::with_std().map_err(|e| format!("Failed to create workspace: {:?}", e))?; + load_mlf_directory(&mut ws, mlf_dir)?; + + ws.resolve() + .map_err(|e| format!("Failed to resolve workspace: {:?}", e))?; + + let mut out = Vec::new(); + for mlf_file in find_files_with_ext(mlf_dir, "mlf")? { + let relative = mlf_file + .strip_prefix(mlf_dir) + .map_err(|e| format!("Failed to strip prefix: {}", e))? + .to_path_buf(); + + let namespace = relative + .with_extension("") + .to_str() + .ok_or("Non-UTF8 path")? + .replace(std::path::MAIN_SEPARATOR, "."); - fn copy_recursive(src: &Path, dst: &Path) -> std::io::Result<()> { - fs::create_dir_all(dst)?; - for entry in fs::read_dir(src)? { - let entry = entry?; - let src_path = entry.path(); - let dst_path = dst.join(entry.file_name()); + let lexicon = ws + .get_lexicon(&namespace) + .ok_or_else(|| format!("Module not found in workspace: {}", namespace))?; - if src_path.is_dir() { - copy_recursive(&src_path, &dst_path)?; - } else { - fs::copy(&src_path, &dst_path)?; - } - } - Ok(()) + let json = generate_lexicon(&namespace, lexicon, &ws); + out.push((relative.with_extension("json"), json)); } - copy_recursive(source_dir, dest_dir) - .map_err(|e| format!("Failed to copy MLF files: {}", e))?; - - let mlf_count = find_mlf_files(dest_dir)?.len(); - println!(" Copied {} MLF files", mlf_count); - - Ok(()) + Ok(out) } -/// Generate JSON from MLF files using `mlf generate lexicon` -fn generate_json_from_mlf(workspace_dir: &Path, output_dir: &Path) -> Result<(), String> { - let lexicons_dir = workspace_dir.join("lexicons"); +/// Recursively find files with the given extension under `dir`. +fn find_files_with_ext(dir: &Path, ext: &str) -> Result, String> { + let mut files = Vec::new(); - if !lexicons_dir.exists() { - return Err(format!("Lexicons directory not found: {}", lexicons_dir.display())); - } - - // Create output directory - fs::create_dir_all(output_dir) - .map_err(|e| format!("Failed to create output directory: {}", e))?; - - // Generate all JSON files at once by passing the lexicons directory - // This allows proper dependency resolution between MLF files - println!(" Generating JSON files..."); - let output = Command::new("mlf") - .arg("generate") - .arg("lexicon") - .arg("-i") - .arg("lexicons") - .arg("-o") - .arg(output_dir) - .current_dir(workspace_dir) - .output() - .map_err(|e| format!("Failed to execute mlf generate: {}", e))?; - - if !output.status.success() { - return Err(format!( - "mlf generate lexicon failed:\n{}", - String::from_utf8_lossy(&output.stderr) - )); - } - - println!(" Generated JSON successfully"); - - Ok(()) -} - -/// Find all .mlf files recursively -fn find_mlf_files(dir: &Path) -> Result, String> { - let mut mlf_files = Vec::new(); - - fn walk_dir(dir: &Path, files: &mut Vec) -> std::io::Result<()> { - if dir.is_dir() { - for entry in fs::read_dir(dir)? { - let entry = entry?; - let path = entry.path(); - if path.is_dir() { - walk_dir(&path, files)?; - } else if path.extension().and_then(|s| s.to_str()) == Some("mlf") { - files.push(path); - } + fn walk(dir: &Path, ext: &str, out: &mut Vec) -> std::io::Result<()> { + if !dir.is_dir() { + return Ok(()); + } + for entry in fs::read_dir(dir)? { + let entry = entry?; + let path = entry.path(); + if path.is_dir() { + walk(&path, ext, out)?; + } else if path.extension().and_then(|s| s.to_str()) == Some(ext) { + out.push(path); } } Ok(()) } - walk_dir(dir, &mut mlf_files).map_err(|e| format!("Failed to walk directory: {}", e))?; - Ok(mlf_files) + walk(dir, ext, &mut files).map_err(|e| format!("Failed to walk directory: {}", e))?; + Ok(files) } #[derive(Debug)] @@ -240,84 +208,62 @@ struct ComparisonStats { failed_lexicons: Vec<(String, String)>, } -/// Compare original JSON with regenerated JSON -fn compare_json_files( +fn compare_regenerated( original_dir: &Path, - generated_dir: &Path, + regenerated: &[(PathBuf, serde_json::Value)], diffs_dir: &Path, ) -> Result { - // Create diffs directory fs::create_dir_all(diffs_dir) .map_err(|e| format!("Failed to create diffs directory: {}", e))?; + let mut stats = ComparisonStats { - total: 0, + total: regenerated.len(), perfect_matches: 0, acceptable_diffs: 0, failures: 0, failed_lexicons: Vec::new(), }; - // Find all JSON files in original directory - let original_files = find_json_files(original_dir)?; - stats.total = original_files.len(); - - println!(" Comparing {} lexicon files...", stats.total); - - for original_file in original_files { - let relative_path = original_file - .strip_prefix(original_dir) - .map_err(|e| format!("Failed to strip prefix: {}", e))?; - - let generated_file = generated_dir.join(relative_path); - - // Extract NSID from path for reporting + for (relative_path, generated) in regenerated { let nsid = relative_path .with_extension("") .to_str() .unwrap() .replace(std::path::MAIN_SEPARATOR, "."); - if !generated_file.exists() { + let original_path = original_dir.join(relative_path); + if !original_path.exists() { stats.failures += 1; - stats.failed_lexicons - .push((nsid.clone(), "Generated file not found".to_string())); - eprintln!(" āœ— {}: Generated file not found", nsid); + stats + .failed_lexicons + .push((nsid, format!("Original file not found: {}", original_path.display()))); continue; } - // Read and parse JSON files - let original_json = fs::read_to_string(&original_file) + let original_text = fs::read_to_string(&original_path) .map_err(|e| format!("Failed to read original: {}", e))?; - let generated_json = fs::read_to_string(&generated_file) - .map_err(|e| format!("Failed to read generated: {}", e))?; - - let original: serde_json::Value = serde_json::from_str(&original_json) + let original: serde_json::Value = serde_json::from_str(&original_text) .map_err(|e| format!("Failed to parse original JSON: {}", e))?; - let generated: serde_json::Value = serde_json::from_str(&generated_json) - .map_err(|e| format!("Failed to parse generated JSON: {}", e))?; - // Compare with allowed differences - match compare_lexicon_json(&original, &generated) { + let generated_text = serde_json::to_string_pretty(generated) + .map_err(|e| format!("Failed to serialise generated JSON: {}", e))?; + + match compare_lexicon_json(&original, generated) { ComparisonResult::Perfect => { stats.perfect_matches += 1; - println!(" āœ“ {} (perfect match)", nsid); } ComparisonResult::AcceptableDifferences(diffs) => { stats.acceptable_diffs += 1; - println!(" āœ“ {} (acceptable diffs: {})", nsid, diffs.join(", ")); - - // Write diff file for acceptable differences - write_diff_file(diffs_dir, &nsid, &original_json, &generated_json, "acceptable") + write_diff_file(diffs_dir, &nsid, &original_text, &generated_text, "acceptable") .unwrap_or_else(|e| eprintln!("Warning: Failed to write diff: {}", e)); + println!(" āœ“ {} (acceptable: {})", nsid, diffs.join(", ")); } ComparisonResult::Failure(reason) => { stats.failures += 1; stats.failed_lexicons.push((nsid.clone(), reason.clone())); - eprintln!(" āœ— {}: {}", nsid, reason); - - // Write diff file for failures - write_diff_file(diffs_dir, &nsid, &original_json, &generated_json, "failure") + write_diff_file(diffs_dir, &nsid, &original_text, &generated_text, "failure") .unwrap_or_else(|e| eprintln!("Warning: Failed to write diff: {}", e)); + eprintln!(" āœ— {}: {}", nsid, reason); } } } @@ -325,29 +271,6 @@ fn compare_json_files( Ok(stats) } -/// Find all JSON files recursively -fn find_json_files(dir: &Path) -> Result, String> { - let mut json_files = Vec::new(); - - fn walk_dir(dir: &Path, files: &mut Vec) -> std::io::Result<()> { - if dir.is_dir() { - for entry in fs::read_dir(dir)? { - let entry = entry?; - let path = entry.path(); - if path.is_dir() { - walk_dir(&path, files)?; - } else if path.extension().and_then(|s| s.to_str()) == Some("json") { - files.push(path); - } - } - } - Ok(()) - } - - walk_dir(dir, &mut json_files).map_err(|e| format!("Failed to walk directory: {}", e))?; - Ok(json_files) -} - #[derive(Debug)] enum ComparisonResult { Perfect, @@ -362,38 +285,23 @@ fn compare_lexicon_json( ) -> ComparisonResult { let mut acceptable_diffs = Vec::new(); - // Strip $type fields (these are often added/removed) let original_stripped = strip_dollar_type(original); let generated_stripped = strip_dollar_type(generated); - // Check if they're identical after stripping $type if original_stripped == generated_stripped { return ComparisonResult::Perfect; } - // Allow $type differences - if has_only_dollar_type_diff(&original_stripped, &generated_stripped) { - acceptable_diffs.push("$type fields".to_string()); - } - - // Check for field ordering differences (same fields, different order) + // The strip above already handled $type-only differences; detect + // field ordering next since our regen emits in declaration order. if has_only_ordering_diff(&original_stripped, &generated_stripped) { acceptable_diffs.push("field ordering".to_string()); return ComparisonResult::AcceptableDifferences(acceptable_diffs); } - // If we have acceptable diffs, return them - if !acceptable_diffs.is_empty() { - return ComparisonResult::AcceptableDifferences(acceptable_diffs); - } - - // Otherwise, it's a failure - ComparisonResult::Failure(format!( - "Structural differences detected" - )) + ComparisonResult::Failure("Structural differences detected".to_string()) } -/// Recursively strip $type fields from JSON fn strip_dollar_type(value: &serde_json::Value) -> serde_json::Value { match value { serde_json::Value::Object(map) => { @@ -412,13 +320,6 @@ fn strip_dollar_type(value: &serde_json::Value) -> serde_json::Value { } } -/// Check if the only difference is $type fields -fn has_only_dollar_type_diff(v1: &serde_json::Value, v2: &serde_json::Value) -> bool { - // After stripping $type, they should be equal - v1 == v2 -} - -/// Write diff files showing differences between original and generated JSON fn write_diff_file( diffs_dir: &Path, nsid: &str, @@ -426,25 +327,20 @@ fn write_diff_file( generated_json: &str, diff_type: &str, ) -> Result<(), String> { - // Create subdirectory based on diff type let type_dir = diffs_dir.join(diff_type); fs::create_dir_all(&type_dir) .map_err(|e| format!("Failed to create diff type directory: {}", e))?; - // Create base filename from NSID let base_filename = nsid.replace('.', "_"); - // Write original JSON let original_path = type_dir.join(format!("{}.original.json", base_filename)); fs::write(&original_path, original_json) .map_err(|e| format!("Failed to write original JSON: {}", e))?; - // Write generated JSON let generated_path = type_dir.join(format!("{}.generated.json", base_filename)); fs::write(&generated_path, generated_json) .map_err(|e| format!("Failed to write generated JSON: {}", e))?; - // Run diff command and save output let diff_path = type_dir.join(format!("{}.diff", base_filename)); let diff_output = Command::new("diff") .arg("-u") @@ -453,24 +349,23 @@ fn write_diff_file( .output() .map_err(|e| format!("Failed to run diff command: {}", e))?; - // diff returns exit code 1 when files differ, which is expected - // Only error if exit code is 2+ (indicates an error running diff) + // diff exit code 1 just means "files differ" — only error on 2+. if diff_output.status.code() == Some(2) { - return Err(format!("diff command error: {}", String::from_utf8_lossy(&diff_output.stderr))); + return Err(format!( + "diff command error: {}", + String::from_utf8_lossy(&diff_output.stderr) + )); } - // Write diff output fs::write(&diff_path, &diff_output.stdout) .map_err(|e| format!("Failed to write diff output: {}", e))?; Ok(()) } -/// Check if the only difference is field ordering in objects fn has_only_ordering_diff(v1: &serde_json::Value, v2: &serde_json::Value) -> bool { match (v1, v2) { (serde_json::Value::Object(map1), serde_json::Value::Object(map2)) => { - // Check if they have the same keys let keys1: HashSet<_> = map1.keys().collect(); let keys2: HashSet<_> = map2.keys().collect(); @@ -478,7 +373,6 @@ fn has_only_ordering_diff(v1: &serde_json::Value, v2: &serde_json::Value) -> boo return false; } - // Check if all values match (recursively) for key in keys1 { let val1 = &map1[key]; let val2 = &map2[key]; @@ -491,7 +385,6 @@ fn has_only_ordering_diff(v1: &serde_json::Value, v2: &serde_json::Value) -> boo true } (serde_json::Value::Array(arr1), serde_json::Value::Array(arr2)) => { - // Arrays must match exactly (order matters) if arr1.len() != arr2.len() { return false; }