Skip to content
Merged
Show file tree
Hide file tree
Changes from all 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
14 changes: 14 additions & 0 deletions Cargo.lock

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

5 changes: 3 additions & 2 deletions bundle/src/files.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ use std::{
};

use chrono::{DateTime, Utc, serde::ts_milliseconds};
use codeowners::{CodeOwners, Owners, OwnersOfPath};
use codeowners::{CodeOwners, Owners, OwnersOfPath, OwnersSource};
use constants::ALLOW_LIST;
use context::junit::junit_path::{
JunitReportFileWithTestRunnerReport, TestRunnerReport, TestRunnerReportStatus,
Expand Down Expand Up @@ -35,11 +35,12 @@ impl FileSetBuilder {
repo_root: T,
junit_paths: &[JunitReportFileWithTestRunnerReport],
codeowners_path: &Option<U>,
codeowners_type: Option<OwnersSource>,
exec_start: Option<SystemTime>,
) -> anyhow::Result<Self> {
let repo_root = repo_root.as_ref();

let codeowners = CodeOwners::find_file(repo_root, codeowners_path);
let codeowners = CodeOwners::find_file(repo_root, codeowners_path, codeowners_type);

let file_set_builder =
Self::file_sets_from_glob(repo_root, junit_paths, codeowners, exec_start)?;
Expand Down
4 changes: 3 additions & 1 deletion cli/src/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
FileSet, FileSetBuilder, FileSetType, INTERNAL_BIN_FILENAME, META_VERSION,
QuarantineBulkTestStatus, bin_parse,
};
use codeowners::CodeOwners;
use codeowners::{CodeOwners, OwnersSource};
use constants::{ENVS_TO_GET, TRUNK_PR_NUMBER_ENV};
use context::{
bazel_bep::{
Expand Down Expand Up @@ -212,13 +212,15 @@
meta: &mut BundleMeta,
junit_path_wrappers: Vec<JunitReportFileWithTestRunnerReport>,
codeowners_path: &Option<U>,
codeowners_type: Option<OwnersSource>,
allow_empty_test_results: bool,
test_run_result: &Option<TestRunResult>,
) -> anyhow::Result<FileSetBuilder> {
let mut file_set_builder = FileSetBuilder::build_file_sets(
&meta.base_props.repo.repo_root,
&junit_path_wrappers,
codeowners_path,
codeowners_type,
test_run_result.as_ref().and_then(|r| r.exec_start),
)?;

Expand Down Expand Up @@ -300,7 +302,7 @@
) -> TestResult {
TestResult {
test_case_runs,
// trunk-ignore(clippy/deprecated)

Check notice on line 305 in cli/src/context.rs

View workflow job for this annotation

GitHub Actions / Trunk Check

trunk(ignore-does-nothing)

[new] trunk-ignore(clippy/deprecated) is not suppressing a lint issue
uploader_metadata: Some(UploaderMetadata {
variant: variant.unwrap_or_default(),
..Default::default()
Expand Down
26 changes: 26 additions & 0 deletions cli/src/upload_command.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
use api::{client::get_api_host, urls::url_for_test_case};
use bundle::{BundleMeta, BundlerUtil, Test, unzip_tarball};
use clap::{ArgAction, Args};
use codeowners::OwnersSource;
use constants::EXIT_SUCCESS;
use context::bazel_bep::common::BepParseResult;
use display::{end_output::EndOutput, message::DisplayMessage};
Expand Down Expand Up @@ -123,6 +124,13 @@
help = "Override the path to a CODEOWNERS file. Used to associate test failures with code owners."
)]
pub codeowners_path: Option<String>,
#[arg(
long,
env = constants::TRUNK_CODEOWNERS_TYPE_ENV,
value_enum,
help = "Specify CODEOWNERS parser type ('github' or 'gitlab'). Optional; when unset, codeowners type will be inferred."
)]
pub codeowners_type: Option<CodeownersType>,
#[arg(
long,
help = "Run commands with the quarantining step. Deprecated, prefer --disable-quarantining, which takes priority over this flag, to control quarantining.",
Expand Down Expand Up @@ -259,6 +267,23 @@
pub show_failure_messages: bool,
}

#[derive(clap::ValueEnum, Clone, Copy, Debug, Default, PartialEq, Eq)]
#[value(rename_all = "lower")]
pub enum CodeownersType {
#[default]
GitHub,
GitLab,
}

impl From<CodeownersType> for OwnersSource {
fn from(value: CodeownersType) -> Self {
match value {
CodeownersType::GitHub => OwnersSource::GitHub,
CodeownersType::GitLab => OwnersSource::GitLab,
}
}
}

fn parse_sha(s: &str) -> anyhow::Result<String> {
if s.len() > 40 {
anyhow::bail!(anyhow::Error::msg(format!(
Expand Down Expand Up @@ -384,6 +409,7 @@
&mut meta,
junit_path_wrappers,
&upload_args.codeowners_path,
upload_args.codeowners_type.map(Into::into),
upload_args.allow_empty_test_results,
&test_run_result,
)?;
Expand Down Expand Up @@ -462,7 +488,7 @@
.quarantine_results
.clone();

// trunk-ignore(clippy/assigning_clones)

Check notice on line 491 in cli/src/upload_command.rs

View workflow job for this annotation

GitHub Actions / Trunk Check

trunk(ignore-does-nothing)

[new] trunk-ignore(clippy/assigning_clones) is not suppressing a lint issue
meta.failed_tests = quarantine_context.failures.clone();

let upload_started_at = chrono::Utc::now();
Expand Down
32 changes: 26 additions & 6 deletions cli/src/validate_command.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ use std::{

use bundle::{FileSet, FileSetBuilder, FileSetTestRunnerReport};
use clap::{ArgAction, Args, arg};
use codeowners::CodeOwners;
use codeowners::{CodeOwners, OwnersSource};
use constants::{EXIT_FAILURE, EXIT_SUCCESS};
use context::{
bazel_bep::{common::BepParseResult, parser::BazelBepParser},
Expand All @@ -31,7 +31,7 @@ use superconsole::{

use crate::{
context::fall_back_to_binary_parse, error_report::InterruptingError,
report_limiting::ValidationReport,
report_limiting::ValidationReport, upload_command::CodeownersType,
};

#[derive(Args, Clone, Debug)]
Expand Down Expand Up @@ -66,6 +66,13 @@ pub struct ValidateArgs {
help = "Override the path to a CODEOWNERS file. Used to validate code ownership associations."
)]
pub codeowners_path: Option<String>,
#[arg(
long,
env = constants::TRUNK_CODEOWNERS_TYPE_ENV,
value_enum,
help = "Specify CODEOWNERS parser type ('github' or 'gitlab'). Optional; when unset, codeowners type will be inferred."
)]
pub codeowners_type: Option<CodeownersType>,
#[arg(
long,
help = "Deprecated (does nothing, left in to avoid breaking existing flows)",
Expand Down Expand Up @@ -456,6 +463,7 @@ pub async fn run_validate(validate_args: ValidateArgs) -> anyhow::Result<Validat
test_reports,
show_warnings: _,
codeowners_path,
codeowners_type,
..
} = validate_args;

Expand Down Expand Up @@ -496,7 +504,13 @@ pub async fn run_validate(validate_args: ValidateArgs) -> anyhow::Result<Validat
),
}
};
validate(junit_file_paths, codeowners_path, bep_validate_result).await
validate(
junit_file_paths,
codeowners_path,
codeowners_type.map(OwnersSource::from),
bep_validate_result,
)
.await
}

type JunitFileToReportAndParseIssues = BTreeMap<
Expand All @@ -514,14 +528,20 @@ type JunitFileToValidation = BTreeMap<String, JunitReportValidation>;
async fn validate(
junit_paths: Vec<JunitReportFileWithTestRunnerReport>,
codeowners_path: Option<String>,
codeowners_type: Option<OwnersSource>,
bep_result: Option<BepValidateResult>,
) -> anyhow::Result<ValidateRunResult> {
let current_dir = std::env::current_dir()
.ok()
.and_then(|p| p.to_str().map(String::from))
.unwrap_or_default();
let file_set_builder =
FileSetBuilder::build_file_sets(&current_dir, &junit_paths, &Option::<&str>::None, None)?;
let file_set_builder = FileSetBuilder::build_file_sets(
&current_dir,
&junit_paths,
&Option::<&str>::None,
None,
None,
)?;
if file_set_builder.no_files_found() {
let msg = "No test output files found to validate";
tracing::warn!(msg);
Expand Down Expand Up @@ -569,7 +589,7 @@ async fn validate(
.collect();
let test_issues = gen_test_issues(&report_validations);

let codeowners = CodeOwners::find_file(&current_dir, &codeowners_path);
let codeowners = CodeOwners::find_file(&current_dir, &codeowners_path, codeowners_type);
let codeowners_issues = gen_codeowners_issues(codeowners, &report_validations);

Ok(ValidateRunResult {
Expand Down
10 changes: 10 additions & 0 deletions cli/tests/common/command_builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ pub struct UploadArgs {
no_upload: Option<bool>,
team: Option<String>,
codeowners_path: Option<String>,
codeowners_type: Option<String>,
disable_quarantining: Option<bool>,
allow_empty_test_results: Option<bool>,
variant: Option<String>,
Expand All @@ -38,6 +39,7 @@ impl UploadArgs {
no_upload: None,
team: None,
codeowners_path: None,
codeowners_type: None,
disable_quarantining: None,
allow_empty_test_results: None,
variant: None,
Expand Down Expand Up @@ -124,6 +126,14 @@ impl UploadArgs {
vec![String::from("--codeowners-path"), codeowners_path]
}),
)
.chain(
self.codeowners_type
.clone()
.into_iter()
.flat_map(|codeowners_type: String| {
vec![String::from("--codeowners-type"), codeowners_type]
}),
)
.chain(
self.disable_quarantining
.into_iter()
Expand Down
1 change: 1 addition & 0 deletions codeowners/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ js-sys = { version = "0.3", optional = true }
pyo3-stub-gen = { version = "0.6.1", optional = true }
tokio = { version = "*", default-features = false, features = ["rt", "macros"] }
tracing = "0.1.41"
tracing-subscriber = { version = "0.3", features = ["env-filter"] }
once_cell = "1.21.3"

[target.'cfg(target_os = "linux")'.dependencies]
Expand Down
62 changes: 56 additions & 6 deletions codeowners/src/codeowners.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
use std::{
fs::File,
io::{Read, Seek},
path::{Path, PathBuf},
sync::Arc,
};
Expand Down Expand Up @@ -83,6 +84,7 @@ impl CodeOwners {
pub fn find_file<T: AsRef<Path>, U: AsRef<Path>>(
repo_root: T,
codeowners_path_cli_option: &Option<U>,
owners_source: Option<OwnersSource>,
) -> Option<Self> {
let cli_option_location = codeowners_path_cli_option
.as_slice()
Expand All @@ -97,13 +99,23 @@ impl CodeOwners {
all_locations.find_map(|location| locate_codeowners(&repo_root, location));

codeowners_path.map(|path| {
let owners_result = File::open(&path)
let owners_result: anyhow::Result<Owners> = File::open(&path)
.map_err(anyhow::Error::from)
.and_then(|file| GitLabOwners::from_reader(&file).map(Owners::GitLabOwners))
.or_else(|_| {
File::open(&path)
.map_err(anyhow::Error::from)
.and_then(|file| GitHubOwners::from_reader(&file).map(Owners::GitHubOwners))
.and_then(|mut file| {
let parse_github = |r: &mut File| {
GitHubOwners::from_reader(r.by_ref()).map(Owners::GitHubOwners)
};
let parse_gitlab = |r: &mut File| {
GitLabOwners::from_reader(r.by_ref()).map(Owners::GitLabOwners)
};
match owners_source {
Some(OwnersSource::GitHub) => parse_github(&mut file),
Some(OwnersSource::GitLab) => parse_gitlab(&mut file),
_ => parse_gitlab(&mut file).or_else(|_| {
file.seek(std::io::SeekFrom::Start(0))?;
parse_github(&mut file)
}),
}
});

if let Err(ref err) = owners_result {
Expand Down Expand Up @@ -322,6 +334,10 @@ where

#[cfg(test)]
mod tests {
use std::fs;
use std::path::Path;
use std::time::{SystemTime, UNIX_EPOCH};

use super::*;

fn make_codeowners_bytes(i: usize, owners_source: Option<OwnersSource>) -> CodeOwnersFile {
Expand Down Expand Up @@ -432,4 +448,38 @@ mod tests {
assert_eq!(owners[0], format!("@user{user_id}"));
}
}

#[test]
fn test_find_file_with_explicit_owners_source() {
let unique = SystemTime::now()
.duration_since(UNIX_EPOCH)
.unwrap()
.as_nanos();
let temp_dir = std::env::temp_dir().join(format!("codeowners-source-test-{unique}"));
fs::create_dir_all(&temp_dir).unwrap();
let codeowners_file = temp_dir.join("CODEOWNERS");
fs::write(
&codeowners_file,
"[DefaultOwners] @gitlab-owner\nREADME.md\n",
)
.unwrap();

let parsed_gitlab =
CodeOwners::find_file(&temp_dir, &Some(Path::new(".")), Some(OwnersSource::GitLab))
.unwrap();
let gitlab_owners = flatten_code_owners(&parsed_gitlab, &"README.md".to_string());
assert_eq!(gitlab_owners, vec!["@gitlab-owner".to_string()]);

let parsed_github =
CodeOwners::find_file(&temp_dir, &Some(Path::new(".")), Some(OwnersSource::GitHub))
.unwrap();
let github_owners = flatten_code_owners(&parsed_github, &"README.md".to_string());
assert!(github_owners.is_empty());

let parsed_default = CodeOwners::find_file(&temp_dir, &Some(Path::new(".")), None).unwrap();
let default_owners = flatten_code_owners(&parsed_default, &"README.md".to_string());
assert_eq!(default_owners, vec!["@gitlab-owner".to_string()]);

fs::remove_dir_all(temp_dir).ok();
}
}
Loading
Loading