Skip to content

Commit

Permalink
Optimize E2E tests build times (#953)
Browse files Browse the repository at this point in the history
See each PR commit for exact detail what's changed.

---------

Signed-off-by: Marek Kaput <[email protected]>
  • Loading branch information
mkaput authored Nov 29, 2023
1 parent 6d8703a commit ff22121
Show file tree
Hide file tree
Showing 12 changed files with 70 additions and 71 deletions.
1 change: 0 additions & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,6 @@ fs_extra = "1"
futures = { version = "0.3", default-features = false, features = ["std", "async-await"] }
gix = ">=0.55"
glob = "0.3"
hyper = "0.14"
ignore = "0.4"
include_dir = "0.7"
indicatif = "0.17"
Expand Down
2 changes: 1 addition & 1 deletion scarb/src/bin/scarb/commands/commands.rs
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ mod tests {
use camino::Utf8Path;

use scarb::core::Config;
use scarb::process::make_executable;
use scarb_test_support::fsx::make_executable;

use super::{list_commands, CommandInfo};

Expand Down
40 changes: 40 additions & 0 deletions scarb/src/internal/fsx.rs
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,46 @@ pub fn copy(from: impl AsRef<Path>, to: impl AsRef<Path>) -> Result<u64> {
}
}

#[cfg(unix)]
pub fn is_executable<P: AsRef<Path>>(path: P) -> bool {
use std::os::unix::prelude::*;
fs::metadata(path)
.map(|metadata| metadata.is_file() && metadata.permissions().mode() & 0o111 != 0)
.unwrap_or(false)
}

#[cfg(windows)]
pub fn is_executable<P: AsRef<Path>>(path: P) -> bool {
path.as_ref().is_file()
}

#[cfg(unix)]
pub fn is_hidden(entry: impl AsRef<Path>) -> bool {
is_hidden_by_dot(entry)
}

#[cfg(windows)]
pub fn is_hidden(entry: impl AsRef<Path>) -> bool {
use std::os::windows::prelude::*;

let is_hidden = fs::metadata(entry.as_ref())
.ok()
.map(|metadata| metadata.file_attributes())
.map(|attributes| (attributes & 0x2) > 0)
.unwrap_or(false);

is_hidden || is_hidden_by_dot(entry)
}

fn is_hidden_by_dot(entry: impl AsRef<Path>) -> bool {
entry
.as_ref()
.file_stem()
.and_then(|s| s.to_str())
.map(|s| s.starts_with('.'))
.unwrap_or(false)
}

pub trait PathUtf8Ext {
fn try_as_utf8(&'_ self) -> Result<&'_ Utf8Path>;

Expand Down
3 changes: 2 additions & 1 deletion scarb/src/ops/subcommands.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,9 @@ use tracing::debug;
use scarb_ui::components::Status;

use crate::core::{Config, Package, ScriptDefinition, Workspace};
use crate::internal::fsx::is_executable;
use crate::ops;
use crate::process::{exec_replace, is_executable};
use crate::process::exec_replace;
use crate::subcommands::{get_env_vars, EXTERNAL_CMD_PREFIX, SCARB_MANIFEST_PATH_ENV};

/// Prepare environment and execute an external subcommand.
Expand Down
3 changes: 1 addition & 2 deletions scarb/src/ops/workspace.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,8 @@ use crate::core::source::SourceId;
use crate::core::workspace::Workspace;
use crate::core::TomlManifest;
use crate::internal::fsx;
use crate::internal::fsx::PathBufUtf8Ext;
use crate::internal::fsx::{is_hidden, PathBufUtf8Ext};
use crate::ops::find_workspace_manifest_path;
use crate::process::is_hidden;
use crate::MANIFEST_FILE_NAME;

#[tracing::instrument(level = "debug", skip(config))]
Expand Down
61 changes: 5 additions & 56 deletions scarb/src/process.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
use std::ffi::OsStr;
use std::io::{BufRead, BufReader, Read};
use std::path::Path;
use std::process::{Command, Stdio};
use std::sync::Arc;
use std::{fmt, thread};
Expand All @@ -11,6 +10,7 @@ use tracing::{debug, debug_span, warn, Span};
use scarb_ui::components::{Spinner, Status};

use crate::core::Config;
pub use crate::internal::fsx::is_executable;

/// Replaces the current process with the target process.
///
Expand All @@ -34,10 +34,11 @@ pub fn exec_replace(cmd: &mut Command) -> Result<()> {

#[cfg(unix)]
mod imp {
use anyhow::{bail, Result};
use std::os::unix::process::CommandExt;
use std::process::Command;

use anyhow::{bail, Result};

pub fn exec_replace(cmd: &mut Command) -> Result<()> {
let err = cmd.exec();
bail!("process did not exit successfully: {err}")
Expand All @@ -46,8 +47,9 @@ mod imp {

#[cfg(windows)]
mod imp {
use anyhow::{bail, Context, Result};
use std::process::Command;

use anyhow::{bail, Context, Result};
use windows_sys::Win32::Foundation::{BOOL, FALSE, TRUE};
use windows_sys::Win32::System::Console::SetConsoleCtrlHandler;

Expand Down Expand Up @@ -144,20 +146,6 @@ pub fn exec(cmd: &mut Command, config: &Config) -> Result<()> {
}
}

#[cfg(unix)]
pub fn is_executable<P: AsRef<Path>>(path: P) -> bool {
use std::fs;
use std::os::unix::prelude::*;
fs::metadata(path)
.map(|metadata| metadata.is_file() && metadata.permissions().mode() & 0o111 != 0)
.unwrap_or(false)
}

#[cfg(windows)]
pub fn is_executable<P: AsRef<Path>>(path: P) -> bool {
path.as_ref().is_file()
}

/// Python's [`shlex.join`] for [`Command`].
///
/// [`shlex.join`]: https://docs.python.org/3/library/shlex.html#shlex.join
Expand Down Expand Up @@ -188,42 +176,3 @@ impl<'a> fmt::Display for ShlexJoin<'a> {
Ok(())
}
}

#[cfg(unix)]
pub fn make_executable(path: &Path) {
use std::fs;
use std::os::unix::prelude::*;
let mut perms = fs::metadata(path).unwrap().permissions();
perms.set_mode(perms.mode() | 0o700);
fs::set_permissions(path, perms).unwrap();
}

#[cfg(windows)]
pub fn make_executable(_path: &Path) {}

#[cfg(unix)]
pub fn is_hidden(entry: impl AsRef<Path>) -> bool {
is_hidden_by_dot(entry)
}

#[cfg(windows)]
pub fn is_hidden(entry: impl AsRef<Path>) -> bool {
use std::os::windows::prelude::*;

let is_hidden = std::fs::metadata(entry.as_ref())
.ok()
.map(|metadata| metadata.file_attributes())
.map(|attributes| (attributes & 0x2) > 0)
.unwrap_or(false);

is_hidden || is_hidden_by_dot(entry)
}

fn is_hidden_by_dot(entry: impl AsRef<Path>) -> bool {
entry
.as_ref()
.file_stem()
.and_then(|s| s.to_str())
.map(|s| s.starts_with('.'))
.unwrap_or(false)
}
2 changes: 1 addition & 1 deletion scarb/tests/scripts.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,9 @@ use assert_fs::prelude::*;
use assert_fs::TempDir;
use indoc::{formatdoc, indoc};

use scarb::process::make_executable;
use scarb_test_support::command::{CommandExt, Scarb};
use scarb_test_support::filesystem::{path_with_temp_dir, write_simple_hello_script};
use scarb_test_support::fsx::make_executable;
use scarb_test_support::project_builder::ProjectBuilder;
use scarb_test_support::workspace_builder::WorkspaceBuilder;

Expand Down
8 changes: 4 additions & 4 deletions scarb/tests/subcommand.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ use indoc::indoc;
use scarb_test_support::cargo::cargo_bin;

use scarb_test_support::command::Scarb;
use scarb_test_support::filesystem::{path_with_temp_dir, write_script, write_simple_hello_script};
use scarb_test_support::filesystem::{path_with_temp_dir, write_simple_hello_script};
use scarb_test_support::project_builder::ProjectBuilder;

#[test]
Expand Down Expand Up @@ -56,7 +56,7 @@ fn list_commands_e2e() {
#[cfg(unix)]
fn env_variables_are_passed() {
let t = TempDir::new().unwrap();
write_script(
scarb_test_support::filesystem::write_script(
"env",
indoc! {
r#"
Expand Down Expand Up @@ -98,7 +98,7 @@ fn env_variables_are_passed() {
#[cfg(unix)]
fn env_scarb_log_is_passed_verbatim() {
let t = TempDir::new().unwrap();
write_script(
scarb_test_support::filesystem::write_script(
"env",
indoc! {
r#"
Expand Down Expand Up @@ -196,7 +196,7 @@ fn can_find_scarb_directory_scripts_without_path() {

#[test]
fn can_list_scarb_directory_scripts() {
let t = assert_fs::TempDir::new().unwrap();
let t = TempDir::new().unwrap();
write_simple_hello_script("hello", &t);

// Set scarb path to folder containing hello script
Expand Down
2 changes: 1 addition & 1 deletion utils/scarb-test-support/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ camino.workspace = true
clap.workspace = true
data-encoding.workspace = true
dunce.workspace = true
hyper.workspace = true
hyper = "*" # Link whatever comes with reqwest.
indoc.workspace = true
itertools.workspace = true
once_cell.workspace = true
Expand Down
3 changes: 2 additions & 1 deletion utils/scarb-test-support/src/filesystem.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,8 @@ use std::{env, iter, vec};
use assert_fs::prelude::*;
use assert_fs::TempDir;
use indoc::indoc;
use scarb::process::make_executable;

use crate::fsx::make_executable;

pub fn write_script(name: &str, script_source: &str, t: &TempDir) {
let script = t.child(format!("scarb-{name}{}", env::consts::EXE_SUFFIX));
Expand Down
13 changes: 12 additions & 1 deletion utils/scarb-test-support/src/fsx.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use std::fs;
use std::fs::File;
use std::io::BufReader;
use std::path::{PathBuf, MAIN_SEPARATOR_STR};
use std::path::{Path, PathBuf, MAIN_SEPARATOR_STR};

use assert_fs::fixture::ChildPath;
use assert_fs::TempDir;
Expand All @@ -15,6 +15,17 @@ pub use internal_fsx::{canonicalize, canonicalize_utf8, PathBufUtf8Ext, PathUtf8
#[path = "../../../scarb/src/internal/fsx.rs"]
mod internal_fsx;

#[cfg(unix)]
pub fn make_executable(path: &Path) {
use std::os::unix::prelude::*;
let mut perms = fs::metadata(path).unwrap().permissions();
perms.set_mode(perms.mode() | 0o700);
fs::set_permissions(path, perms).unwrap();
}

#[cfg(windows)]
pub fn make_executable(_path: &Path) {}

pub trait AssertFsUtf8Ext {
fn utf8_path(&self) -> &Utf8Path;
}
Expand Down
3 changes: 1 addition & 2 deletions utils/scarb-test-support/src/workspace_builder.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
use crate::project_builder::{DepBuilder, ProjectBuilder};
use assert_fs::prelude::*;
use scarb::MANIFEST_FILE_NAME;
use toml_edit::{Array, Document, Item, Value};

#[derive(Default)]
Expand Down Expand Up @@ -59,6 +58,6 @@ impl WorkspaceBuilder {
manifest.push_str(&self.manifest_extra);
}

t.child(MANIFEST_FILE_NAME).write_str(&manifest).unwrap();
t.child("Scarb.toml").write_str(&manifest).unwrap();
}
}

0 comments on commit ff22121

Please sign in to comment.