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

feat: runtime test expectation file #70

Open
wants to merge 10 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 8 commits
Commits
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
55 changes: 54 additions & 1 deletion Cargo.lock

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

6 changes: 5 additions & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ cc.workspace = true
clap.workspace = true
console.workspace = true
kdl.workspace = true
include_dir.workspace = true
libloading.workspace = true
linked-hash-map.workspace = true
miette.workspace = true
Expand All @@ -32,7 +33,8 @@ thiserror.workspace = true
tokio.workspace = true
tracing.workspace = true
tracing-subscriber.workspace = true
include_dir = "0.7.4"
toml.workspace = true


[build-dependencies]
built.workspace = true
Expand Down Expand Up @@ -60,6 +62,7 @@ camino = { version = "1.1.7", features = ["serde1"] }
cc = { version = "1.1.0" }
clap = { version = "4.5.4", features = ["cargo", "wrap_help", "derive"] }
console = "0.15.8"
include_dir = "0.7.4"
kdl = "4.6.0"
libloading = "0.7.3"
linked-hash-map = { version = "0.5.6", features = ["serde", "serde_impl"] }
Expand All @@ -73,6 +76,7 @@ serde = { version = "1.0.136", features = ["derive"] }
serde_json = "1.0.83"
thiserror = "1.0.30"
tokio = { version = "1.37.0", features = ["full", "tracing"] }
toml = "0.8.14"
tracing = "0.1.40"
tracing-subscriber = { version = "0.3.18", features = ["env-filter"] }
# build
Expand Down
28 changes: 28 additions & 0 deletions abi-cafe-rules-example.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
# Here are some example annotations for test expecations

# this test fails on this platform, with this toolchain pairing
[targets.x86_64-pc-windows-msvc."simple::cc_calls_rustc"]
fail = "check"

# this test has random results on this platform, whenever rustc is the caller (callee also supported)
[targets.x86_64-pc-windows-msvc."simple::rustc_caller"]
random = true

# whenever this test involves cc, only link it, and expect linking to fail
[targets.x86_64-pc-windows-msvc."EmptyStruct::cc_toolchain"]
run = "link"
fail = "link"

# any repr(c) version of this test fails to run
[targets.x86_64-unknown-linux-gnu."simple::repr_c"]
busted = "run"

# for this pairing, with the rust calling convention, only generate the test, and expect it to work
[targets.x86_64-unknown-linux-gnu."simple::rustc_calls_rustc::conv_rust"]
run = "generate"
pass = "generate"

# can match all tests with leading ::
[targets.x86_64-unknown-linux-gnu."::rustc_calls_rustc"]
run = "generate"
pass = "generate"
19 changes: 19 additions & 0 deletions include/harness/abi-cafe-rules.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
# i128 types are fake on windows so this is all random garbage that might
# not even compile, but that datapoint is a little interesting/useful
# so let's keep running them and just ignore the result for now.
#
# Anyone who cares about this situation more can make the expectations more precise.
[targets.x86_64-pc-windows-msvc."i128::cc_toolchain"]
random = true
[targets.x86_64-pc-windows-msvc."u128::cc_toolchain"]
random = true

# FIXME: investigate why this is failing to build
[targets.x86_64-pc-windows-msvc."EmptyStruct::cc_toolchain"]
busted = "build"
[targets.x86_64-pc-windows-msvc."EmptyStructInside::cc_toolchain"]
busted = "build"

# CI GCC is too old to support _Float16
[targets.x86_64-unknown-linux-gnu."f16::conv_c"]
Copy link
Contributor

Choose a reason for hiding this comment

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

Having some way to support arbitrary cfg() conditions would be really useful to indicate that something is broken on a certain architecture or OS without having to list every single target with this architecture or OS you want to test.

Copy link
Owner Author

Choose a reason for hiding this comment

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

Hmm yeah I agree, but am definitely running out of steam to build out this feature myself, and am hoping this is an acceptable MVP X3

A few things:

  • I specifically made it so you can do [targets."*"."blhablhablha"] if you really don't care
  • I expect most users are going to be copy-pasting what the new suggestion system spits out, so verbosity is hopefully less of an issue?
    • And having "too many" records makes it easier to granularly update them if one platform is fixed?

Copy link
Owner Author

Choose a reason for hiding this comment

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

Partially there is some sense of like, when bringing up a new platform, I think you do want some expectations tabula-rasa? But I might be wrong.

Copy link
Contributor

Choose a reason for hiding this comment

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

If you have a test failing on x86_64-unknown-linux-gnu, it is almost guaranteed to fail on all x86_64 unix targets as those share a single abi document. The only cases I know of where the OS actually matters are unix vs windows-msvc vs windows-gnu(?) on x86_64 and (non-apple) unix vs apple vs windows arm64ec on arm64.

Copy link
Owner Author

Choose a reason for hiding this comment

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

Hmm are there any libraries y'all compiler devs have for evaling this stuff at runtime?

Copy link
Contributor

@bjorn3 bjorn3 Jul 14, 2024

Choose a reason for hiding this comment

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

You could try using the target-lexicon crate (as used by cranelift and by extension cg_clif). It has a default_calling_convention method which returns the calling convention used for extern "C" (also mostly applies to extern "Rust") on the respective target. It doesn't yet handle arm64ec though.

random = true
2 changes: 1 addition & 1 deletion kdl-script/src/parse.rs
Original file line number Diff line number Diff line change
Expand Up @@ -218,7 +218,7 @@ pub enum Repr {
Transparent,
}

#[derive(Debug, Copy, Clone, PartialEq, Eq, Hash, serde::Serialize)]
#[derive(Debug, Copy, Clone, PartialEq, Eq, PartialOrd, Ord, Hash, serde::Serialize)]
pub enum LangRepr {
Rust,
C,
Expand Down
20 changes: 20 additions & 0 deletions src/cli.rs
Original file line number Diff line number Diff line change
Expand Up @@ -130,12 +130,27 @@ struct Cli {
#[clap(long)]
add_tests: Option<Utf8PathBuf>,

/// Add the test expectations at the given path
///
/// (If not specified we'll look for a file called abi-cafe-rules.toml in the working dir)
///
/// Note that there are already builtin rules (disabled with `--disable-builtin-riles`),
Gankra marked this conversation as resolved.
Show resolved Hide resolved
/// and it would be nice for rules to be upstreamed so everyone can benefit!
#[clap(long)]
rules: Option<Utf8PathBuf>,

/// disable the builtin tests
///
/// See also `--add-tests`
#[clap(long)]
disable_builtin_tests: bool,

/// disable the builtin rules
///
/// See also `--add-rules`
#[clap(long)]
disable_builtin_rules: bool,

/// deprecated, does nothing (we always procgen now)
#[clap(long, hide = true)]
procgen_tests: bool,
Expand All @@ -154,7 +169,9 @@ pub fn make_app() -> Config {
output_format,
add_rustc_codegen_backend,
add_tests,
rules,
disable_builtin_tests,
disable_builtin_rules,
// unimplemented
select_vals: _,
key: _,
Expand Down Expand Up @@ -228,12 +245,14 @@ Hint: Try using `--pairs {name}_calls_rustc` or `--pairs rustc_calls_{name}`.
let out_dir = target_dir.join("temp");
let generated_src_dir = target_dir.join("generated_impls");
let runtime_test_input_dir = add_tests;
let runtime_rules_file = rules.unwrap_or_else(|| "abi-cafe-rules.toml".into());

let paths = Paths {
target_dir,
out_dir,
generated_src_dir,
runtime_test_input_dir,
runtime_rules_file,
};
Config {
output_format,
Expand All @@ -248,6 +267,7 @@ Hint: Try using `--pairs {name}_calls_rustc` or `--pairs rustc_calls_{name}`.
run_selections,
minimizing_write_impl,
disable_builtin_tests,
disable_builtin_rules,
paths,
}
}
2 changes: 2 additions & 0 deletions src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@ pub enum GenerateError {
#[error(transparent)]
#[diagnostic(transparent)]
KdlScriptError(#[from] kdl_script::KdlScriptError),
#[error(transparent)]
TomlError(#[from] toml::de::Error),
/// Used to signal we just skipped it
#[error("<skipped>")]
Skipped,
Expand Down
7 changes: 6 additions & 1 deletion src/files.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ pub struct Paths {
pub out_dir: Utf8PathBuf,
pub generated_src_dir: Utf8PathBuf,
pub runtime_test_input_dir: Option<Utf8PathBuf>,
pub runtime_rules_file: Utf8PathBuf,
}
impl Paths {
pub fn harness_dylib_main_file(&self) -> Utf8PathBuf {
Expand Down Expand Up @@ -89,12 +90,16 @@ pub fn load_file(file: &File) -> String {
clean_newlines(string)
}

pub fn tests() -> &'static Dir<'static> {
pub fn static_tests() -> &'static Dir<'static> {
INCLUDES
.get_dir("tests")
.expect("includes didn't contain ./test")
}

pub fn static_rules() -> String {
get_file("harness/abi-cafe-rules.toml")
}

fn clean_newlines(input: &str) -> String {
input.replace('\r', "")
}
2 changes: 1 addition & 1 deletion src/harness/build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,8 @@ use camino::Utf8Path;
use tracing::info;

use crate::error::*;
use crate::harness::report::*;
use crate::harness::test::*;
use crate::report::*;
use crate::*;

impl TestHarness {
Expand Down
1 change: 0 additions & 1 deletion src/harness/check.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ use kdl_script::types::Ty;
use tracing::{error, info};

use crate::error::*;
use crate::report::*;
use crate::*;

impl TestHarness {
Expand Down
34 changes: 13 additions & 21 deletions src/harness/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,12 @@ mod build;
mod check;
mod generate;
mod read;
pub mod report;
mod run;
pub mod test;
pub mod vals;

pub use read::{find_tests, spawn_read_test};
pub use read::{find_test_rules, find_tests, spawn_read_test};
pub use run::TestBuffer;

pub type Memoized<K, V> = Mutex<SortedMap<K, Arc<OnceCell<V>>>>;
Expand All @@ -30,6 +31,7 @@ pub struct TestHarness {
paths: Paths,
toolchains: Toolchains,
tests: SortedMap<TestId, Arc<Test>>,
test_rules: Vec<ExpectFile>,
tests_with_vals: Memoized<(TestId, ValueGeneratorKind), Arc<TestWithVals>>,
tests_with_toolchain:
Memoized<(TestId, ValueGeneratorKind, ToolchainId), Arc<TestWithToolchain>>,
Expand All @@ -39,11 +41,16 @@ pub struct TestHarness {
}

impl TestHarness {
pub fn new(tests: SortedMap<TestId, Arc<Test>>, cfg: &Config) -> Self {
pub fn new(
test_rules: Vec<ExpectFile>,
tests: SortedMap<TestId, Arc<Test>>,
cfg: &Config,
) -> Self {
let toolchains = toolchains::create_toolchains(cfg);
Self {
paths: cfg.paths.clone(),
tests,
test_rules,
toolchains,
tests_with_vals: Default::default(),
tests_with_toolchain: Default::default(),
Expand Down Expand Up @@ -109,13 +116,6 @@ impl TestHarness {
.clone();
Ok(output)
}
pub fn get_test_rules(&self, test_key: &TestKey) -> TestRules {
let caller = self.toolchains[&test_key.caller].clone();
let callee = self.toolchains[&test_key.callee].clone();

get_test_rules(test_key, &*caller, &*callee)
}

pub fn spawn_test(
self: Arc<Self>,
rt: &tokio::runtime::Runtime,
Expand Down Expand Up @@ -262,26 +262,18 @@ impl TestHarness {
WriteImpl::HarnessCallback => {
// Do nothing, implicit default
}
WriteImpl::Print => {
output.push_str(separator);
output.push_str("print");
}
WriteImpl::Assert => {
output.push_str(separator);
output.push_str("assert");
}
WriteImpl::Noop => {
other => {
output.push_str(separator);
output.push_str("noop");
output.push_str(&other.to_string())
}
}
match val_generator {
ValueGeneratorKind::Graffiti => {
// Do nothing, implicit default
}
ValueGeneratorKind::Random { seed } => {
other => {
output.push_str(separator);
output.push_str(&format!("random{seed}"));
output.push_str(&other.to_string())
}
}
output
Expand Down
Loading
Loading