Skip to content

Commit

Permalink
Bumps rust toolchain and resolves linting errors
Browse files Browse the repository at this point in the history
  • Loading branch information
TAGraves committed Oct 4, 2024
1 parent 1e8764f commit 01a62d0
Show file tree
Hide file tree
Showing 25 changed files with 87 additions and 242 deletions.
3 changes: 3 additions & 0 deletions .cargo/config → .cargo/config.toml
Original file line number Diff line number Diff line change
Expand Up @@ -8,3 +8,6 @@ ABQ_WORKSPACE_DIR = { value = "", relative = true }
# easily hit the number of open FDs (especially on MacOS) during our tests.
# Revisit this when we've made everything async on the worker side.
RUST_TEST_THREADS = "1"

[workspace]
resolver = "2"
24 changes: 12 additions & 12 deletions .github/workflows/build_and_upload.yml
Original file line number Diff line number Diff line change
Expand Up @@ -5,26 +5,26 @@ on:
workflow_dispatch:
inputs:
ref:
description: 'ref to build'
description: "ref to build"
required: true
type: string
release_channel:
description: 'release channel'
description: "release channel"
required: true
type: choice
options:
- "v1"
- "unstable"
- "v1"
- "unstable"

# for automatic release from main on the unstable release channel
workflow_call:
inputs:
ref:
description: 'ref to build'
description: "ref to build"
required: true
type: string
release_channel:
description: 'release channel'
description: "release channel"
required: true
type: string
secrets:
Expand Down Expand Up @@ -61,24 +61,24 @@ jobs:
deprecated-platform: linux_x86-64
os: linux
architecture: x86_64
cross-target: 'x86_64-unknown-linux-musl'
cross-target: "x86_64-unknown-linux-musl"
install-musl-tools: true
- runs-on: ubuntu-latest
deprecated-platform: linux_aarch64
os: linux
architecture: aarch64
cross-target: 'aarch64-unknown-linux-musl'
cross-target: "aarch64-unknown-linux-musl"
container: messense/rust-musl-cross:aarch64-musl@sha256:777bd4c61179c38dc213bb8472500584646d28fd4a7c3e0b30b9ef70cb446d58
- runs-on: macos-11 # use an older version for broader osx support
deprecated-platform: darwin_x86-64
os: darwin
architecture: x86_64
cross-target: ''
cross-target: ""
- runs-on: macos-11 # first OS X to support arm64 -- so the first os for cross compilation
deprecated-platform: darwin_aarch64
os: darwin
architecture: aarch64
cross-target: 'aarch64-apple-darwin'
cross-target: "aarch64-apple-darwin"
runs-on: ${{ matrix.runs-on }}
container: ${{ matrix.container }}
outputs:
Expand Down Expand Up @@ -122,7 +122,7 @@ jobs:
- name: Install Rust toolchain
uses: rwx-research/rust-toolchain@abq
with:
toolchain: 1.65.0
toolchain: 1.81.0
target: ${{ matrix.cross-target }}

# We don't build a musl ABQ on MacOS
Expand All @@ -132,7 +132,7 @@ jobs:
sudo apt-get install -y musl-tools
- name: Build release
if: '!matrix.cross-target'
if: "!matrix.cross-target"
run: cargo build --release --all-features

- name: Build release
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/test_and_package_development.yml
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ jobs:
- name: Install Rust toolchain
uses: rwx-research/rust-toolchain@abq
with:
toolchain: 1.65.0
toolchain: 1.81.0
target: ${{ env.RUST_TARGET }}
components: clippy, rustfmt

Expand Down
1 change: 1 addition & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
[workspace]
resolver = "2"

members = [
"crates/abq_cli",
Expand Down
20 changes: 10 additions & 10 deletions crates/abq_cli/src/args.rs
Original file line number Diff line number Diff line change
Expand Up @@ -150,12 +150,12 @@ pub enum Command {
///
/// The remote persistence options are:{n}
/// - s3: files are remotely persisted to an S3 bucket. Requires `ABQ_REMOTE_PERSISTENCE_S3_BUCKET`
/// and `ABQ_REMOTE_PERSISTENCE_S3_KEY_PREFIX` to be set as well. AWS credentials and region
/// information are read from the environment, using the standard AWS environment variable
/// support (https://docs.aws.amazon.com/sdkref/latest/guide/environment-variables.html).{n}
/// and `ABQ_REMOTE_PERSISTENCE_S3_KEY_PREFIX` to be set as well. AWS credentials and region
/// information are read from the environment, using the standard AWS environment variable
/// support (https://docs.aws.amazon.com/sdkref/latest/guide/environment-variables.html).{n}
///
/// - custom: files are remotely persisted by calling a provided executable. See
/// `--remote-persistence-command` for more information.{n}
/// `--remote-persistence-command` for more information.{n}
#[clap(long, required = false, env(ENV_REMOTE_PERSISTENCE_STRATEGY))]
remote_persistence_strategy: Option<RemotePersistenceStrategy>,

Expand All @@ -169,7 +169,7 @@ pub enum Command {
///
/// Where{n}
/// - <mode> is either "store" or "load", depending on whether the file should be stored
/// into the remote location, or loaded from the remote location.{n}
/// into the remote location, or loaded from the remote location.{n}
/// - <file-type> is "manifest", "results", or "run_state".{n}
/// - <run-id> is the run ID of the test suite run.{n}
/// - <local-path> is the path to the file on the local filesystem.{n}
Expand Down Expand Up @@ -331,7 +331,7 @@ pub enum Command {
/// Options:{n}
///- by-test: distribute the next test to any worker.{n}
///- by-file: distribute all tests in a file to the same worker. This ensures that expensive per-file shared setups or
/// teardowns will run only once on one worker, however it may cause tests to be less evenly distributed.
/// teardowns will run only once on one worker, however it may cause tests to be less evenly distributed.
///
/// Note: The Jest & Playwright test frameworks run with a by-file strategy regardless of the value of this flag.
#[clap(long, default_value = "by-test")]
Expand All @@ -341,8 +341,8 @@ pub enum Command {
///
/// Options:{n}
///- auto: try to emit colors unless the output channel is detected
/// not to be a TTY, if (on Windows) the console isn't available, if NO_COLOR is set, if
/// TERM is set to `dumb`, amongst other heuristics.
/// not to be a TTY, if (on Windows) the console isn't available, if NO_COLOR is set, if
/// TERM is set to `dumb`, amongst other heuristics.
///- never: don't emit colors.
#[clap(long, default_value = "auto")]
color: ColorPreference,
Expand Down Expand Up @@ -411,8 +411,8 @@ pub enum Command {
///
/// Options:{n}
///- auto: try to emit colors unless the output channel is detected
/// not to be a TTY, if (on Windows) the console isn't available, if NO_COLOR is set, if
/// TERM is set to `dumb`, amongst other heuristics.
/// not to be a TTY, if (on Windows) the console isn't available, if NO_COLOR is set, if
/// TERM is set to `dumb`, amongst other heuristics.
///- never: don't emit colors.
#[clap(long, default_value = "auto")]
color: ColorPreference,
Expand Down
18 changes: 4 additions & 14 deletions crates/abq_cli/src/instance.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ use signal_hook::consts::TERM_SIGNALS;
use signal_hook::iterator::Signals;
use std::net::{IpAddr, SocketAddr};
use std::thread;
use tempfile::TempDir;
use tokio_cron_scheduler::JobScheduler;

use thiserror::Error;
Expand Down Expand Up @@ -280,12 +279,7 @@ enum AbqLocator {
queue_negotiator: QueueNegotiatorHandle,
server_addr: SocketAddr,
},
Local(Abq, EphemeralAbqGuards),
}

struct EphemeralAbqGuards {
_manifests_path: TempDir,
_results_path: TempDir,
Local(Abq),
}

#[derive(Debug, Error)]
Expand All @@ -312,14 +306,14 @@ impl AbqInstance {
AbqLocator::Remote {
queue_negotiator, ..
} => *queue_negotiator,
AbqLocator::Local(abq, _) => abq.get_negotiator_handle(),
AbqLocator::Local(abq) => abq.get_negotiator_handle(),
}
}

pub fn server_addr(&self) -> SocketAddr {
match &self.locator {
AbqLocator::Remote { server_addr, .. } => *server_addr,
AbqLocator::Local(abq, _) => abq.server_addr(),
AbqLocator::Local(abq) => abq.server_addr(),
}
}

Expand Down Expand Up @@ -359,13 +353,9 @@ impl AbqInstance {
config.server_options = ServerOptions::new(server_auth, server_tls);

let queue = Abq::start(config).await;
let guards = EphemeralAbqGuards {
_manifests_path: manifests_path,
_results_path: results_path,
};

AbqInstance {
locator: AbqLocator::Local(queue, guards),
locator: AbqLocator::Local(queue),
client_options: ClientOptions::new(client_auth, client_tls),
}
}
Expand Down
14 changes: 7 additions & 7 deletions crates/abq_cli/tests/cli.rs
Original file line number Diff line number Diff line change
Expand Up @@ -475,7 +475,7 @@ test_all_network_config_options! {
exit_status,
} = Abq::new(name)
.args(args)
.working_dir(&testdata_project("jest/npm-jest-project"))
.working_dir(testdata_project("jest/npm-jest-project"))
.run();

assert!(exit_status.success());
Expand All @@ -496,7 +496,7 @@ test_all_network_config_options! {
exit_status,
} = Abq::new(name)
.args(args)
.working_dir(&testdata_project("jest/npm-jest-project"))
.working_dir(testdata_project("jest/npm-jest-project"))
.run();

assert!(exit_status.success());
Expand Down Expand Up @@ -688,7 +688,7 @@ test_all_network_config_options! {
let CmdOutput { stdout, stderr, exit_status } =
Abq::new(name.to_string() + "_worker0")
.args(test_args(0))
.working_dir(&testdata_project("jest/npm-jest-project"))
.working_dir(testdata_project("jest/npm-jest-project"))
.run();
assert!(!exit_status.success());
assert!(stdout.contains("-- Test Timeout --"), "STDOUT:\n{}STDERR:\n{}", stdout, stderr);
Expand Down Expand Up @@ -718,7 +718,7 @@ test_all_network_config_options! {
exit_status,
} = Abq::new(name)
.args(test_args)
.working_dir(&testdata_project("jest/npm-jest-project-with-failures"))
.working_dir(testdata_project("jest/npm-jest-project-with-failures"))
.run();

let code = exit_status.code().expect("process killed");
Expand Down Expand Up @@ -1542,7 +1542,7 @@ fn test_grouping_without_failures() {

let stdout = String::from_utf8_lossy(&stdout).to_string();
let stderr = String::from_utf8_lossy(&stderr).to_string();
let stdouts = vec![&stdout];
let stdouts = [&stdout];
assert_sum_of_run_tests(stdouts.iter().map(|s| s.as_str()), 64);
assert_sum_of_run_test_failures(stdouts.iter().map(|s| s.as_str()), 0);
assert_sum_of_run_test_retries(stdouts.iter().map(|s| s.as_str()), 0);
Expand Down Expand Up @@ -1672,7 +1672,7 @@ fn test_grouping_with_failures_without_retries() {

let stdout = String::from_utf8_lossy(&stdout).to_string();
let stderr = String::from_utf8_lossy(&stderr).to_string();
let stdouts = vec![&stdout];
let stdouts = [&stdout];
assert_sum_of_run_tests(stdouts.iter().map(|s| s.as_str()), 64);
assert_sum_of_run_test_failures(stdouts.iter().map(|s| s.as_str()), 64);
assert_sum_of_run_test_retries(stdouts.iter().map(|s| s.as_str()), 0);
Expand Down Expand Up @@ -1802,7 +1802,7 @@ fn test_grouping_failures_retries() {

let stdout = String::from_utf8_lossy(&stdout).to_string();
let stderr = String::from_utf8_lossy(&stderr).to_string();
let stdouts = vec![&stdout];
let stdouts = [&stdout];
assert_sum_of_run_tests(stdouts.iter().map(|s| s.as_str()), 64);
assert_sum_of_run_test_failures(stdouts.iter().map(|s| s.as_str()), 64);
assert_sum_of_run_test_retries(stdouts.iter().map(|s| s.as_str()), 64);
Expand Down
2 changes: 1 addition & 1 deletion crates/abq_queue/src/job_queue.rs
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ impl JobQueue {
entity_tag: Tag,
suggested_batch_size: NonZeroUsize,
) -> impl ExactSizeIterator<Item = &(WorkerTest, GroupId)> + '_ {
let suggested_batch_size = suggested_batch_size.get() as usize;
let suggested_batch_size = suggested_batch_size.get();
let queue_len = self.queue.len();

// If the start index was past the end of the queue, return fast
Expand Down
1 change: 1 addition & 0 deletions crates/abq_queue/src/persistence/manifest/fs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,7 @@ impl FilesystemPersistor {
tokio::task::spawn_blocking(move || {
let fi = std::fs::OpenOptions::new()
.create(true)
.truncate(false)
.read(true)
.write(true)
.open(path)
Expand Down
5 changes: 2 additions & 3 deletions crates/abq_queue/src/persistence/remote/fake.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,6 @@ pub struct FakePersister {
on_try_load_run_state: Arc<OnTryLoadRunState>,
}

#[track_caller]
pub async fn unreachable(_x: PersistenceKind, _y: RunId, _z: PathBuf) -> OpaqueResult<()> {
unreachable!()
}
Expand Down Expand Up @@ -220,11 +219,11 @@ impl RemotePersistence for OneWriteFakePersister {
_run_id: &RunId,
_run_state: SerializableRunState,
) -> OpaqueResult<()> {
unimplemented!("FakePersister does not support storing run state.");
Err("FakePersister does not support storing run state.".located(here!()))
}

async fn try_load_run_state(&self, _run_id: &RunId) -> OpaqueResult<LoadedRunState> {
unimplemented!("FakePersister does not support loading run state.");
Err("FakePersister does not support loading run state.".located(here!()))
}

fn boxed_clone(&self) -> Box<dyn RemotePersistence + Send + Sync> {
Expand Down
2 changes: 1 addition & 1 deletion crates/abq_queue/src/persistence/results/fs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -410,7 +410,7 @@ impl PersistResults for FilesystemPersistor {

async fn write_packed_line(fi: &mut File, packed: Vec<u8>) -> OpaqueResult<()> {
fi.write_all(&packed).await.located(here!())?;
fi.write_all(&[b'\n']).await.located(here!())?;
fi.write_all(b"\n").await.located(here!())?;
fi.flush().await.located(here!())
}

Expand Down
2 changes: 0 additions & 2 deletions crates/abq_queue/src/persistence/results/test_utils.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
#![cfg(test)]

use std::sync::Arc;

use abq_utils::{
Expand Down
7 changes: 2 additions & 5 deletions crates/abq_queue/src/queue.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ use abq_utils::net_protocol::queue::{
AssociatedTestResults, CancelReason, GroupId, NativeRunnerInfo, NegotiatorInfo, Request,
TestResultsResponse, TestSpec, TestStrategy,
};
use abq_utils::net_protocol::results::{self, OpaqueLazyAssociatedTestResults};
use abq_utils::net_protocol::results::{self};
use abq_utils::net_protocol::runners::{Manifest, MetadataMap, StdioOutput};
use abq_utils::net_protocol::work_server::{self, RetryManifestResponse};
use abq_utils::net_protocol::workers::{
Expand Down Expand Up @@ -1546,6 +1546,7 @@ impl Drop for Abq {
if self.active {
// Our user never called shutdown; try to perform a clean exit.
// We can't do anything with an error, since this is a drop.
#[allow(clippy::let_underscore_future)]
let _ = self.shutdown();
}
}
Expand Down Expand Up @@ -2315,10 +2316,6 @@ impl QueueServer {
entity: Entity,
mut stream: Box<dyn net_async::ServerStream>,
) -> OpaqueResult<()> {
enum Response {
One(TestResultsResponse),
Chunk(OpaqueLazyAssociatedTestResults),
}
let results_cell = match queues.get_read_results_cell(&run_id).located(here!()) {
Ok(state) => match state {
ReadResultsState::ReadFromCell(cell) => cell,
Expand Down
1 change: 0 additions & 1 deletion crates/abq_queue/src/queue/test_utils.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
#![cfg(test)]
use abq_test_utils::one_nonzero_usize;
use abq_utils::{
net_protocol::{
Expand Down
14 changes: 3 additions & 11 deletions crates/abq_queue/tests/integration.rs
Original file line number Diff line number Diff line change
Expand Up @@ -71,14 +71,6 @@ struct Run(usize);
#[derive(Clone, Copy, Debug, PartialEq, Eq, Hash)]
struct Wid(usize);

/// External party ID
#[derive(Clone, Copy, Debug, PartialEq, Eq, Hash)]
struct ExternId(usize);

/// ID of a spawned action
#[derive(Clone, Copy, Debug, PartialEq, Eq, Hash)]
struct SpawnId(usize);

/// External dependencies that must be preserved while a test is ongoing.
struct QueueExtDeps {
_manifests_path: TempDir,
Expand Down Expand Up @@ -377,7 +369,7 @@ enum TestResultsOutcome {
Results(OpaqueLazyAssociatedTestResults),
Error(String),
Pending,
OutstandingRunners(Vec<net_protocol::entity::Tag>),
OutstandingRunners,
}

#[allow(clippy::type_complexity)]
Expand Down Expand Up @@ -728,8 +720,8 @@ async fn run_test(server: Server, steps: Steps<'_>) {
TestResultsOutcome::Results(results)
}
Pending => TestResultsOutcome::Pending,
RunInProgress { active_runners } => {
TestResultsOutcome::OutstandingRunners(active_runners)
RunInProgress { .. } => {
TestResultsOutcome::OutstandingRunners
}
Error(s) => TestResultsOutcome::Error(s),
};
Expand Down
Loading

0 comments on commit 01a62d0

Please sign in to comment.