From d7bb2a362d731951489a167b5122ada48f4be8b8 Mon Sep 17 00:00:00 2001 From: zackees Date: Sun, 23 Aug 2026 17:16:37 -0700 Subject: [PATCH] chore(build-engine): build test fixtures from the canonical path segments MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Sixth ratchet batch for FastLED/fbuild#1349. Allowlist 20 -> 15, and `fbuild-build-engine` is clear. Six files of test fixtures that spelled `/.fbuild/build/...` by hand: `build_info`, the two compile-database suites, `compiler_tests`, `linker`, and `compiler`. Each now assembles its paths from `FBUILD_DIR_NAME` / `BUILD_DIR_NAME`. Fixtures are worth ratcheting for a reason that is easy to miss: a hand-spelled fixture keeps passing after the layout changes. It asserts against the shape the test author typed, not the shape the code produces, so the suite goes on reporting green while describing a directory nothing writes to. That is the same failure the production sites have, minus the symptom. `compiler_tests` had more sites than the first pass surfaced — the framework cache root and three toolchain/include paths under `~/.fbuild/packages/` — all found by the lint rather than by reading. Lint crate 0.1.5 -> 0.1.6, checked against the version this branch starts from rather than against main. Co-Authored-By: Claude Opus 5 (1M context) --- crates/fbuild-build-engine/src/build_info.rs | 4 +- .../src/compile_database/tests/clang.rs | 37 ++++++++++++++----- .../src/compile_database/tests/generate.rs | 18 +++++++-- .../fbuild-build-engine/src/compiler_tests.rs | 37 +++++++++++++++---- crates/fbuild-build-engine/src/linker.rs | 9 ++++- dylints/ban_raw_fbuild_path/Cargo.toml | 2 +- dylints/ban_raw_fbuild_path/src/allowlist.txt | 5 --- 7 files changed, 83 insertions(+), 29 deletions(-) diff --git a/crates/fbuild-build-engine/src/build_info.rs b/crates/fbuild-build-engine/src/build_info.rs index e7c1f19be..5f219ee02 100644 --- a/crates/fbuild-build-engine/src/build_info.rs +++ b/crates/fbuild-build-engine/src/build_info.rs @@ -812,7 +812,9 @@ mod tests { fn find_build_info_walks_up_from_elf_dir() { let tmp = tempfile::TempDir::new().unwrap(); let project = tmp.path(); - let build_dir = project.join(".fbuild").join("build").join("uno"); + let build_dir = fbuild_paths::get_project_fbuild_dir(project) + .join(fbuild_paths::BUILD_DIR_NAME) + .join("uno"); std::fs::create_dir_all(&build_dir).unwrap(); // Emit at the project root (where platformio.ini would live). let info = sample_info(); diff --git a/crates/fbuild-build-engine/src/compile_database/tests/clang.rs b/crates/fbuild-build-engine/src/compile_database/tests/clang.rs index 724c6b1ed..c0ab59125 100644 --- a/crates/fbuild-build-engine/src/compile_database/tests/clang.rs +++ b/crates/fbuild-build-engine/src/compile_database/tests/clang.rs @@ -7,6 +7,25 @@ use crate::compile_database::{ CompileDatabase, CompileEntry, TargetArchitecture, translate_flags_for_clang, }; +/// Fixture build directory, assembled from the canonical segments so these +/// tests cannot describe a layout the resolver no longer produces +/// (FastLED/fbuild#1349). +fn fixture_dir() -> String { + format!( + "/project/{}/{}/env/src", + fbuild_paths::FBUILD_DIR_NAME, + fbuild_paths::BUILD_DIR_NAME + ) +} + +fn fixture_src() -> String { + format!("{}/sketch.ino.cpp", fixture_dir()) +} + +fn fixture_obj() -> String { + format!("{}/sketch.ino.cpp.o", fixture_dir()) +} + #[test] fn test_target_triples() { assert_eq!(TargetArchitecture::Xtensa.target_triple(), "xtensa-esp-elf"); @@ -260,13 +279,13 @@ fn ino_cpp_template_entry() -> CompileEntry { "--target=xtensa-esp-elf".to_string(), "-Isrc".to_string(), "-c".to_string(), - "/project/.fbuild/build/env/src/sketch.ino.cpp".to_string(), + fixture_src().as_str().to_string(), "-o".to_string(), - "/project/.fbuild/build/env/src/sketch.ino.cpp.o".to_string(), + fixture_obj().as_str().to_string(), ], directory: "/project".to_string(), - file: "/project/.fbuild/build/env/src/sketch.ino.cpp".to_string(), - output: Some("/project/.fbuild/build/env/src/sketch.ino.cpp.o".to_string()), + file: fixture_src().as_str().to_string(), + output: Some(fixture_obj().as_str().to_string()), } } @@ -277,7 +296,7 @@ fn test_swap_ino_entries_for_raw_replaces_generated_entry() { let ino_preludes = vec![( PathBuf::from("/project/src/sketch.ino"), - PathBuf::from("/project/.fbuild/build/env/src/sketch.ino.prelude.h"), + PathBuf::from(format!("{}/sketch.ino.prelude.h", fixture_dir())), )]; let swapped = db.swap_ino_entries_for_raw(&ino_preludes); @@ -298,7 +317,7 @@ fn test_swap_ino_entries_for_raw_replaces_generated_entry() { .unwrap(); assert_eq!( entry.arguments[include_idx + 1], - "/project/.fbuild/build/env/src/sketch.ino.prelude.h" + format!("{}/sketch.ino.prelude.h", fixture_dir()).as_str() ); assert!( entry @@ -323,11 +342,11 @@ fn test_swap_ino_entries_for_raw_multi_tab() { let ino_preludes = vec![ ( PathBuf::from("/project/src/main.ino"), - PathBuf::from("/project/.fbuild/build/env/src/main.ino.prelude.h"), + PathBuf::from(format!("{}/main.ino.prelude.h", fixture_dir())), ), ( PathBuf::from("/project/src/a_tab.ino"), - PathBuf::from("/project/.fbuild/build/env/src/a_tab.ino.prelude.h"), + PathBuf::from(format!("{}/a_tab.ino.prelude.h", fixture_dir())), ), ]; let swapped = db.swap_ino_entries_for_raw(&ino_preludes); @@ -372,7 +391,7 @@ fn test_swap_ino_entries_for_raw_no_generated_entry_leaves_db_untouched() { let ino_preludes = vec![( PathBuf::from("/project/src/sketch.ino"), - PathBuf::from("/project/.fbuild/build/env/src/sketch.ino.prelude.h"), + PathBuf::from(format!("{}/sketch.ino.prelude.h", fixture_dir())), )]; let swapped = db.swap_ino_entries_for_raw(&ino_preludes); assert_eq!(swapped.entries.len(), 1); diff --git a/crates/fbuild-build-engine/src/compile_database/tests/generate.rs b/crates/fbuild-build-engine/src/compile_database/tests/generate.rs index 4d19c3f7a..2f19970b0 100644 --- a/crates/fbuild-build-engine/src/compile_database/tests/generate.rs +++ b/crates/fbuild-build-engine/src/compile_database/tests/generate.rs @@ -355,13 +355,21 @@ fn test_generate_entries_file_is_source_not_build() { &[], &[], &[PathBuf::from("/project/src/main.cpp")], - Path::new("/project/.fbuild/build/esp32/src"), + Path::new(&format!( + "/project/{}/{}/esp32/src", + fbuild_paths::FBUILD_DIR_NAME, + fbuild_paths::BUILD_DIR_NAME + )), Path::new("/project"), ); assert_eq!(entries[0].file, "/project/src/main.cpp"); // Output should be in the build dir assert!( - entries[0].output.as_ref().unwrap().contains(".fbuild"), + entries[0] + .output + .as_ref() + .unwrap() + .contains(fbuild_paths::FBUILD_DIR_NAME), "output should be in build dir: {:?}", entries[0].output ); @@ -437,7 +445,11 @@ fn test_generate_entries_include_flags_preserved_verbatim() { // source-tree paths, not build-dir paths. let include_flags = vec![ "-I/project/src".to_string(), // source tree ✓ - "-I/home/user/.fbuild/build/esp32/libs/fastled/src".to_string(), // cache path ✗ + format!( + "-I/home/user/{}/{}/esp32/libs/fastled/src", + fbuild_paths::FBUILD_DIR_NAME, + fbuild_paths::BUILD_DIR_NAME + ), // cache path ✗ "-I/framework/cores/esp32".to_string(), // framework ✓ ]; let entries = generate_entries( diff --git a/crates/fbuild-build-engine/src/compiler_tests.rs b/crates/fbuild-build-engine/src/compiler_tests.rs index 05fa8d5df..96a4a32a1 100644 --- a/crates/fbuild-build-engine/src/compiler_tests.rs +++ b/crates/fbuild-build-engine/src/compiler_tests.rs @@ -35,7 +35,7 @@ async fn compile_path_contract_pairs_cwd_and_output_arg_for_282() { // Workspace shape mirrors CI: /.fbuild/build//quick/core let workspace = tmp_canon.join("proj_for_282"); let core = workspace - .join(".fbuild") + .join(fbuild_paths::FBUILD_DIR_NAME) .join("build") .join("x") .join("quick") @@ -322,7 +322,11 @@ fn test_needs_rebuild_resolves_relative_depfile_deps_against_workspace() { let tmp = tempfile::TempDir::new().unwrap(); let ws = tmp.path(); let src_dir = ws.join("src"); - let build_dir = ws.join(".fbuild/build/demo/release/src"); + let build_dir = ws.join(format!( + "{}/{}/demo/release/src", + fbuild_paths::FBUILD_DIR_NAME, + fbuild_paths::BUILD_DIR_NAME + )); std::fs::create_dir_all(&src_dir).unwrap(); std::fs::create_dir_all(&build_dir).unwrap(); @@ -337,7 +341,11 @@ fn test_needs_rebuild_resolves_relative_depfile_deps_against_workspace() { // compile cwd is the workspace root. std::fs::write( &dep, - ".fbuild/build/demo/release/src/main.cpp.o: src/main.cpp src/config.h\n", + format!( + "{}/{}/demo/release/src/main.cpp.o: src/main.cpp src/config.h\n", + fbuild_paths::FBUILD_DIR_NAME, + fbuild_paths::BUILD_DIR_NAME + ), ) .unwrap(); std::thread::sleep(std::time::Duration::from_millis(20)); @@ -438,8 +446,12 @@ fn test_build_rebuild_signature_for_workspace_outside_paths_keep_legacy_normaliz // to the project-independent normalization — unchanged behavior. let tmp_a = tempfile::tempdir().unwrap(); let tmp_b = tempfile::tempdir().unwrap(); - let global_a = tmp_a.path().join(".fbuild/cache/framework/cores/arduino"); - let global_b = tmp_b.path().join(".fbuild/cache/framework/cores/arduino"); + let cache_rel = format!( + "{}/cache/framework/cores/arduino", + fbuild_paths::FBUILD_DIR_NAME + ); + let global_a = tmp_a.path().join(&cache_rel); + let global_b = tmp_b.path().join(&cache_rel); let ws_a = tmp_a.path().join("proj-a"); let ws_b = tmp_b.path().join("other-name-proj-b"); std::fs::create_dir_all(&ws_a).unwrap(); @@ -501,7 +513,10 @@ fn test_build_rebuild_signature_ignores_absolute_compiler_path() { &[], ); let sig_b = build_rebuild_signature( - Path::new("/home/runner/.fbuild/packages/toolchain-atmelavr/bin/avr-gcc"), + Path::new(&format!( + "/home/runner/{}/packages/toolchain-atmelavr/bin/avr-gcc", + fbuild_paths::FBUILD_DIR_NAME + )), &flags, &[], &[], @@ -536,11 +551,17 @@ fn test_build_rebuild_signature_changes_when_compiler_name_changes() { fn test_build_rebuild_signature_ignores_attached_include_root() { let flags_a = vec![ "-I/tmp/ws-a/project/include".to_string(), - "-I/home/runner/.fbuild/packages/framework-arduinoavr/cores/arduino".to_string(), + format!( + "-I/home/runner/{}/packages/framework-arduinoavr/cores/arduino", + fbuild_paths::FBUILD_DIR_NAME + ), ]; let flags_b = vec![ "-I/tmp/ws-b/project/include".to_string(), - "-I/Users/runner/.fbuild/packages/framework-arduinoavr/cores/arduino".to_string(), + format!( + "-I/Users/runner/{}/packages/framework-arduinoavr/cores/arduino", + fbuild_paths::FBUILD_DIR_NAME + ), ]; let sig_a = build_rebuild_signature( diff --git a/crates/fbuild-build-engine/src/linker.rs b/crates/fbuild-build-engine/src/linker.rs index 9f303d1ff..763b1cf15 100644 --- a/crates/fbuild-build-engine/src/linker.rs +++ b/crates/fbuild-build-engine/src/linker.rs @@ -606,8 +606,13 @@ mod tests { /// FastLED/fbuild#1267, FastLED/FastLED#3867. #[test] fn link_cwd_is_absolute_output_dir_when_all_paths_absolute() { - let out = abs("proj/.fbuild/build/release"); - let paths = [abs("proj/.fbuild/build/release/sketch.o"), abs("sdk/ld")]; + let rel = format!( + "proj/{}/{}/release", + fbuild_paths::FBUILD_DIR_NAME, + fbuild_paths::BUILD_DIR_NAME + ); + let out = abs(&rel); + let paths = [abs(&format!("{rel}/sketch.o")), abs("sdk/ld")]; assert_eq!( link_cwd_for(&out, &paths), Some(out.as_path()), diff --git a/dylints/ban_raw_fbuild_path/Cargo.toml b/dylints/ban_raw_fbuild_path/Cargo.toml index 86bb11704..9da0f2960 100644 --- a/dylints/ban_raw_fbuild_path/Cargo.toml +++ b/dylints/ban_raw_fbuild_path/Cargo.toml @@ -3,7 +3,7 @@ name = "ban_raw_fbuild_path" # Bump the version to bust the dylint .so cache when allowlist.txt # changes (setup-soldr's dylint-cache key hashes the manifest but not # src/allowlist.txt). Same convention ban_manual_slash_normalize follows. -version = "0.1.5" +version = "0.1.6" description = "Ban raw '.fbuild' path literals outside fbuild-paths" edition = "2021" publish = false diff --git a/dylints/ban_raw_fbuild_path/src/allowlist.txt b/dylints/ban_raw_fbuild_path/src/allowlist.txt index d9d01be6f..2de366a35 100644 --- a/dylints/ban_raw_fbuild_path/src/allowlist.txt +++ b/dylints/ban_raw_fbuild_path/src/allowlist.txt @@ -38,11 +38,6 @@ crates/fbuild-cli/src/cli/args.rs # Each line below is a file that spells `.fbuild` by hand today. Removing # a line is the unit of progress on #1349; adding one is not allowed. -crates/fbuild-build-engine/src/build_info.rs -crates/fbuild-build-engine/src/compile_database/tests/clang.rs -crates/fbuild-build-engine/src/compile_database/tests/generate.rs -crates/fbuild-build-engine/src/compiler_tests.rs -crates/fbuild-build-engine/src/linker.rs crates/fbuild-build/src/compile_many.rs crates/fbuild-build/tests/avr_build.rs crates/fbuild-build/tests/clangd_check_parity.rs