Skip to content

Commit

Permalink
Add tracing support to mdtest (#14935)
Browse files Browse the repository at this point in the history
## Summary

This PR extends the mdtest configuration with a `log` setting that can
be any of:

* `true`: Enables tracing
* `false`: Disables tracing (default)
* String: An ENV_FILTER similar to `RED_KNOT_LOG`

```toml
log = true
```

Closes #13865

## Test Plan

I changed a test and tried `log=true`, `log=false`, and `log=INFO`
  • Loading branch information
MichaReiser authored Dec 13, 2024
1 parent 1c8f356 commit f52b1f4
Show file tree
Hide file tree
Showing 14 changed files with 94 additions and 60 deletions.
4 changes: 4 additions & 0 deletions crates/red_knot_python_semantic/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -53,5 +53,9 @@ tempfile = { workspace = true }
quickcheck = { version = "1.0.3", default-features = false }
quickcheck_macros = { version = "1.0.0" }

[features]
serde = ["ruff_db/serde", "dep:serde"]

[lints]
workspace = true

40 changes: 39 additions & 1 deletion crates/red_knot_python_semantic/src/python_version.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ use std::fmt;
/// Unlike the `TargetVersion` enums in the CLI crates,
/// this does not necessarily represent a Python version that we actually support.
#[derive(Debug, Clone, Copy, Eq, PartialEq, Ord, PartialOrd, Hash)]
#[cfg_attr(feature = "serde", derive(serde::Serialize))]
pub struct PythonVersion {
pub major: u8,
pub minor: u8,
Expand Down Expand Up @@ -68,3 +67,42 @@ impl fmt::Display for PythonVersion {
write!(f, "{major}.{minor}")
}
}

#[cfg(feature = "serde")]
impl<'de> serde::Deserialize<'de> for PythonVersion {
fn deserialize<D>(deserializer: D) -> Result<Self, D::Error>
where
D: serde::Deserializer<'de>,
{
let as_str = String::deserialize(deserializer)?;

if let Some((major, minor)) = as_str.split_once('.') {
let major = major
.parse()
.map_err(|err| serde::de::Error::custom(format!("invalid major version: {err}")))?;
let minor = minor
.parse()
.map_err(|err| serde::de::Error::custom(format!("invalid minor version: {err}")))?;

Ok((major, minor).into())
} else {
let major = as_str.parse().map_err(|err| {
serde::de::Error::custom(format!(
"invalid python-version: {err}, expected: `major.minor`"
))
})?;

Ok((major, 0).into())
}
}
}

#[cfg(feature = "serde")]
impl serde::Serialize for PythonVersion {
fn serialize<S>(&self, serializer: S) -> Result<S::Ok, S::Error>
where
S: serde::Serializer,
{
serializer.serialize_str(&self.to_string())
}
}
6 changes: 2 additions & 4 deletions crates/red_knot_test/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,9 @@ authors.workspace = true
license.workspace = true

[dependencies]
red_knot_python_semantic = { workspace = true }
red_knot_python_semantic = { workspace = true, features = ["serde"] }
red_knot_vendored = { workspace = true }
ruff_db = { workspace = true }
ruff_db = { workspace = true, features = ["testing"] }
ruff_index = { workspace = true }
ruff_python_trivia = { workspace = true }
ruff_source_file = { workspace = true }
Expand All @@ -30,7 +30,5 @@ smallvec = { workspace = true }
serde = { workspace = true }
toml = { workspace = true }

[dev-dependencies]

[lints]
workspace = true
2 changes: 2 additions & 0 deletions crates/red_knot_test/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -241,6 +241,8 @@ python-version = "3.10"
This configuration will apply to all tests in the same section, and all nested sections within that
section. Nested sections can override configurations from their parent sections.

See [`MarkdownTestConfig`](https://github.com/astral-sh/ruff/blob/main/crates/red_knot_test/src/config.rs) for the full list of supported configuration options.

## Documentation of tests

Arbitrary Markdown syntax (including of course normal prose paragraphs) is permitted (and ignored by
Expand Down
29 changes: 24 additions & 5 deletions crates/red_knot_test/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,26 +3,45 @@
//! following limited structure:
//!
//! ```toml
//! log = true # or log = "red_knot=WARN"
//! [environment]
//! python-version = "3.10"
//! ```
use anyhow::Context;
use red_knot_python_semantic::PythonVersion;
use serde::Deserialize;

#[derive(Deserialize)]
#[derive(Deserialize, Debug, Default, Clone)]
#[serde(rename_all = "kebab-case", deny_unknown_fields)]
pub(crate) struct MarkdownTestConfig {
pub(crate) environment: Environment,
pub(crate) environment: Option<Environment>,

pub(crate) log: Option<Log>,
}

impl MarkdownTestConfig {
pub(crate) fn from_str(s: &str) -> anyhow::Result<Self> {
toml::from_str(s).context("Error while parsing Markdown TOML config")
}

pub(crate) fn python_version(&self) -> Option<PythonVersion> {
self.environment.as_ref().and_then(|env| env.python_version)
}
}

#[derive(Deserialize)]
#[serde(rename_all = "kebab-case")]
#[derive(Deserialize, Debug, Default, Clone)]
#[serde(rename_all = "kebab-case", deny_unknown_fields)]
pub(crate) struct Environment {
pub(crate) python_version: String,
/// Python version to assume when resolving types.
pub(crate) python_version: Option<PythonVersion>,
}

#[derive(Deserialize, Debug, Clone)]
#[serde(untagged)]
pub(crate) enum Log {
/// Enable logging with tracing when `true`.
Bool(bool),
/// Enable logging and only show filters that match the given [env-filter](https://docs.rs/tracing-subscriber/latest/tracing_subscriber/filter/struct.EnvFilter.html)
Filter(String),
}
9 changes: 8 additions & 1 deletion crates/red_knot_test/src/lib.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
use crate::config::Log;
use camino::Utf8Path;
use colored::Colorize;
use parser as test_parser;
Expand All @@ -7,6 +8,7 @@ use ruff_db::diagnostic::{Diagnostic, ParseDiagnostic};
use ruff_db::files::{system_path_to_file, File, Files};
use ruff_db::parsed::parsed_module;
use ruff_db::system::{DbWithTestSystem, SystemPathBuf};
use ruff_db::testing::{setup_logging, setup_logging_with_filter};
use ruff_source_file::LineIndex;
use ruff_text_size::TextSize;
use salsa::Setter;
Expand Down Expand Up @@ -42,9 +44,14 @@ pub fn run(path: &Utf8Path, long_title: &str, short_title: &str, test_name: &str
continue;
}

let _tracing = test.configuration().log.as_ref().and_then(|log| match log {
Log::Bool(enabled) => enabled.then(setup_logging),
Log::Filter(filter) => setup_logging_with_filter(filter),
});

Program::get(&db)
.set_python_version(&mut db)
.to(test.python_version());
.to(test.configuration().python_version().unwrap_or_default());

// Remove all files so that the db is in a "fresh" state.
db.memory_file_system().remove_all();
Expand Down
30 changes: 7 additions & 23 deletions crates/red_knot_test/src/parser.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,7 @@
use std::sync::LazyLock;

use anyhow::{bail, Context};
use anyhow::bail;
use memchr::memchr2;
use red_knot_python_semantic::PythonVersion;
use regex::{Captures, Match, Regex};
use rustc_hash::{FxHashMap, FxHashSet};

Expand Down Expand Up @@ -74,8 +73,8 @@ impl<'m, 's> MarkdownTest<'m, 's> {
self.files.iter()
}

pub(crate) fn python_version(&self) -> PythonVersion {
self.section.python_version
pub(crate) fn configuration(&self) -> &MarkdownTestConfig {
&self.section.config
}
}

Expand Down Expand Up @@ -125,7 +124,7 @@ struct Section<'s> {
title: &'s str,
level: u8,
parent_id: Option<SectionId>,
python_version: PythonVersion,
config: MarkdownTestConfig,
}

#[newtype_index]
Expand Down Expand Up @@ -222,7 +221,7 @@ impl<'s> Parser<'s> {
title,
level: 0,
parent_id: None,
python_version: PythonVersion::default(),
config: MarkdownTestConfig::default(),
});
Self {
sections,
Expand Down Expand Up @@ -305,7 +304,7 @@ impl<'s> Parser<'s> {
title,
level: header_level.try_into()?,
parent_id: Some(parent),
python_version: self.sections[parent].python_version,
config: self.sections[parent].config.clone(),
};

if self.current_section_files.is_some() {
Expand Down Expand Up @@ -398,23 +397,8 @@ impl<'s> Parser<'s> {
bail!("Multiple TOML configuration blocks in the same section are not allowed.");
}

let config = MarkdownTestConfig::from_str(code)?;
let python_version = config.environment.python_version;

let parts = python_version
.split('.')
.map(str::parse)
.collect::<Result<Vec<_>, _>>()
.context(format!(
"Invalid 'python-version' component: '{python_version}'"
))?;

if parts.len() != 2 {
bail!("Invalid 'python-version': expected MAJOR.MINOR, got '{python_version}'.",);
}

let current_section = &mut self.sections[self.stack.top()];
current_section.python_version = PythonVersion::from((parts[0], parts[1]));
current_section.config = MarkdownTestConfig::from_str(code)?;

self.current_section_has_config = true;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,10 +22,7 @@ WorkspaceMetadata(
],
settings: WorkspaceSettings(
program: ProgramSettings(
python_version: PythonVersion(
major: 3,
minor: 9,
),
python_version: "3.9",
search_paths: SearchPathSettings(
extra_paths: [],
src_root: "/app",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,10 +22,7 @@ WorkspaceMetadata(
],
settings: WorkspaceSettings(
program: ProgramSettings(
python_version: PythonVersion(
major: 3,
minor: 9,
),
python_version: "3.9",
search_paths: SearchPathSettings(
extra_paths: [],
src_root: "/app",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,10 +22,7 @@ WorkspaceMetadata(
],
settings: WorkspaceSettings(
program: ProgramSettings(
python_version: PythonVersion(
major: 3,
minor: 9,
),
python_version: "3.9",
search_paths: SearchPathSettings(
extra_paths: [],
src_root: "/app",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,10 +22,7 @@ WorkspaceMetadata(
],
settings: WorkspaceSettings(
program: ProgramSettings(
python_version: PythonVersion(
major: 3,
minor: 9,
),
python_version: "3.9",
search_paths: SearchPathSettings(
extra_paths: [],
src_root: "/app",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,10 +35,7 @@ WorkspaceMetadata(
],
settings: WorkspaceSettings(
program: ProgramSettings(
python_version: PythonVersion(
major: 3,
minor: 9,
),
python_version: "3.9",
search_paths: SearchPathSettings(
extra_paths: [],
src_root: "/app",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,10 +48,7 @@ WorkspaceMetadata(
],
settings: WorkspaceSettings(
program: ProgramSettings(
python_version: PythonVersion(
major: 3,
minor: 9,
),
python_version: "3.9",
search_paths: SearchPathSettings(
extra_paths: [],
src_root: "/app",
Expand Down
4 changes: 2 additions & 2 deletions crates/ruff_db/src/testing.rs
Original file line number Diff line number Diff line change
Expand Up @@ -158,7 +158,7 @@ impl LoggingBuilder {
.parse()
.expect("Hardcoded directive to be valid"),
),
hierarchical: true,
hierarchical: false,
}
}

Expand All @@ -167,7 +167,7 @@ impl LoggingBuilder {

Some(Self {
filter,
hierarchical: true,
hierarchical: false,
})
}

Expand Down

0 comments on commit f52b1f4

Please sign in to comment.