Skip to content

Commit

Permalink
Removing more unwraps from FFI layer (#18370)
Browse files Browse the repository at this point in the history 
Clears out more of #13608.

Key changes:

- Removed unwraps from FFI layer, mainly in `nodes.rs`.
- Removed redundant `clone().try_into().unwrap()` that converted a
`Process` to a `Process` in the `remote_tests.rs`. This seems to have
been legacy from when `Process` was actually MEPR.
- Renamed `input_files` to `inputs` in the `InputDigests` struct, inline
with the `immutable_inputs` attribute.

---------

Co-authored-by: Stu Hood <stuhood@gmail.com>
  • Loading branch information
@seve-martinez @stuhood
seve-martinez and stuhood authored Mar 7, 2023
1 parent 133646e commit d88e258
Show file tree
Hide file tree
Showing 9 changed files with 97 additions and 166 deletions.
11 changes: 4 additions & 7 deletions src/rust/engine/options/src/env_tests.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,13 +32,10 @@ fn test_display() {
#[test]
fn test_scope() {
let env = env([("PANTS_PYTHON_EXAMPLE", "true")]);
assert_eq!(
true,
env
.get_bool(&option_id!(["python"], "example"))
.unwrap()
.unwrap()
);
assert!(env
.get_bool(&option_id!(["python"], "example"))
.unwrap()
.unwrap());
}

#[test]
Expand Down
2 changes: 1 addition & 1 deletion src/rust/engine/process_execution/docker/src/docker.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -393,7 +393,7 @@ impl<'a> process_execution::CommandRunner for CommandRunner<'a> {
let exclusive_spawn = prepare_workdir(
workdir.path().to_owned(),
&req,
req.input_digests.input_files.clone(),
req.input_digests.inputs.clone(),
self.store.clone(),
self.executor.clone(),
&named_caches,
Expand Down
2 changes: 1 addition & 1 deletion src/rust/engine/process_execution/pe_nailgun/src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -209,7 +209,7 @@ impl process_execution::CommandRunner for CommandRunner {
let exclusive_spawn = prepare_workdir(
nailgun_process.workdir_path().to_owned(),
&client_req,
client_req.input_digests.input_files.clone(),
client_req.input_digests.inputs.clone(),
self.store.clone(),
self.executor.clone(),
&self.named_caches,
Expand Down
2 changes: 1 addition & 1 deletion src/rust/engine/process_execution/pe_nailgun/src/nailgun_pool.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -368,7 +368,7 @@ impl NailgunProcess {
prepare_workdir(
workdir.path().to_owned(),
&startup_options,
startup_options.input_digests.input_files.clone(),
startup_options.input_digests.inputs.clone(),
store.clone(),
executor.clone(),
named_caches,
Expand Down
149 changes: 38 additions & 111 deletions src/rust/engine/process_execution/remote/src/remote_tests.rs
View file
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
// Copyright 2022 Pants project contributors (see CONTRIBUTORS.md).
// Licensed under the Apache License, Version 2.0 (see LICENSE).
use std::collections::{BTreeMap, BTreeSet, HashSet};
use std::convert::TryInto;
use std::path::{Path, PathBuf};
use std::time::Duration;

Expand Down Expand Up @@ -909,15 +908,9 @@ async fn successful_with_only_call_to_execute() {
let mock_server = {
let EntireExecuteRequest {
execute_request, ..
} = process_execution::make_execute_request(
&execute_request.clone().try_into().unwrap(),
None,
None,
&store,
None,
)
.await
.unwrap();
} = process_execution::make_execute_request(&execute_request, None, None, &store, None)
.await
.unwrap();

mock::execution_server::TestServer::new(
mock::execution_server::MockExecution::new(vec![ExpectedAPICall::Execute {
Expand Down Expand Up @@ -960,15 +953,9 @@ async fn successful_after_reconnect_with_wait_execution() {
let mock_server = {
let EntireExecuteRequest {
execute_request, ..
} = process_execution::make_execute_request(
&execute_request.clone().try_into().unwrap(),
None,
None,
&store,
None,
)
.await
.unwrap();
} = process_execution::make_execute_request(&execute_request, None, None, &store, None)
.await
.unwrap();

mock::execution_server::TestServer::new(
mock::execution_server::MockExecution::new(vec![
Expand Down Expand Up @@ -1015,15 +1002,9 @@ async fn successful_after_reconnect_from_retryable_error() {
let mock_server = {
let EntireExecuteRequest {
execute_request, ..
} = process_execution::make_execute_request(
&execute_request.clone().try_into().unwrap(),
None,
None,
&store,
None,
)
.await
.unwrap();
} = process_execution::make_execute_request(&execute_request, None, None, &store, None)
.await
.unwrap();

let execute_request_2 = execute_request.clone();

Expand Down Expand Up @@ -1080,15 +1061,9 @@ async fn creates_executing_workunit() {
let mock_server = {
let EntireExecuteRequest {
execute_request, ..
} = process_execution::make_execute_request(
&execute_request.clone().try_into().unwrap(),
None,
None,
&store,
None,
)
.await
.unwrap();
} = process_execution::make_execute_request(&execute_request, None, None, &store, None)
.await
.unwrap();

mock::execution_server::TestServer::new(
mock::execution_server::MockExecution::new(vec![ExpectedAPICall::Execute {
Expand Down Expand Up @@ -1260,15 +1235,9 @@ async fn server_sending_triggering_timeout_with_deadline_exceeded() {
let mock_server = {
let EntireExecuteRequest {
execute_request, ..
} = process_execution::make_execute_request(
&execute_request.clone().try_into().unwrap(),
None,
None,
&store,
None,
)
.await
.unwrap();
} = process_execution::make_execute_request(&execute_request, None, None, &store, None)
.await
.unwrap();

mock::execution_server::TestServer::new(
mock::execution_server::MockExecution::new(vec![ExpectedAPICall::Execute {
Expand Down Expand Up @@ -1314,15 +1283,9 @@ async fn sends_headers() {
let mock_server = {
let EntireExecuteRequest {
execute_request, ..
} = process_execution::make_execute_request(
&execute_request.clone().try_into().unwrap(),
None,
None,
&store,
None,
)
.await
.unwrap();
} = process_execution::make_execute_request(&execute_request, None, None, &store, None)
.await
.unwrap();

mock::execution_server::TestServer::new(
mock::execution_server::MockExecution::new(vec![ExpectedAPICall::Execute {
Expand Down Expand Up @@ -1516,15 +1479,9 @@ async fn ensure_inline_stdio_is_stored() {

let EntireExecuteRequest {
execute_request, ..
} = process_execution::make_execute_request(
&echo_roland_request().try_into().unwrap(),
None,
None,
&store,
None,
)
.await
.unwrap();
} = process_execution::make_execute_request(&echo_roland_request(), None, None, &store, None)
.await
.unwrap();

mock::execution_server::TestServer::new(
mock::execution_server::MockExecution::new(vec![ExpectedAPICall::Execute {
Expand Down Expand Up @@ -1603,7 +1560,7 @@ async fn bad_result_bytes() {
mock::execution_server::TestServer::new(
mock::execution_server::MockExecution::new(vec![ExpectedAPICall::Execute {
execute_request: process_execution::make_execute_request(
&execute_request.clone().try_into().unwrap(),
&execute_request,
None,
None,
&store,
Expand Down Expand Up @@ -1651,15 +1608,9 @@ async fn initial_response_error() {

let EntireExecuteRequest {
execute_request, ..
} = process_execution::make_execute_request(
&execute_request.clone().try_into().unwrap(),
None,
None,
&store,
None,
)
.await
.unwrap();
} = process_execution::make_execute_request(&execute_request, None, None, &store, None)
.await
.unwrap();

mock::execution_server::TestServer::new(
mock::execution_server::MockExecution::new(vec![ExpectedAPICall::Execute {
Expand Down Expand Up @@ -1706,15 +1657,9 @@ async fn initial_response_missing_response_and_error() {

let EntireExecuteRequest {
execute_request, ..
} = process_execution::make_execute_request(
&execute_request.clone().try_into().unwrap(),
None,
None,
&store,
None,
)
.await
.unwrap();
} = process_execution::make_execute_request(&execute_request, None, None, &store, None)
.await
.unwrap();

mock::execution_server::TestServer::new(
mock::execution_server::MockExecution::new(vec![ExpectedAPICall::Execute {
Expand Down Expand Up @@ -1752,15 +1697,9 @@ async fn fails_after_retry_limit_exceeded() {
let mock_server = {
let EntireExecuteRequest {
execute_request, ..
} = process_execution::make_execute_request(
&execute_request.clone().try_into().unwrap(),
None,
None,
&store,
None,
)
.await
.unwrap();
} = process_execution::make_execute_request(&execute_request, None, None, &store, None)
.await
.unwrap();

mock::execution_server::TestServer::new(
mock::execution_server::MockExecution::new(vec![
Expand Down Expand Up @@ -1816,15 +1755,9 @@ async fn fails_after_retry_limit_exceeded_with_stream_close() {
let op_name = "foo-bar".to_owned();
let EntireExecuteRequest {
execute_request, ..
} = process_execution::make_execute_request(
&execute_request.clone().try_into().unwrap(),
None,
None,
&store,
None,
)
.await
.unwrap();
} = process_execution::make_execute_request(&execute_request, None, None, &store, None)
.await
.unwrap();

mock::execution_server::TestServer::new(
mock::execution_server::MockExecution::new(vec![
Expand Down Expand Up @@ -1899,15 +1832,9 @@ async fn execute_missing_file_uploads_if_known() {

let EntireExecuteRequest {
execute_request, ..
} = process_execution::make_execute_request(
&cat_roland_request().try_into().unwrap(),
None,
None,
&store,
None,
)
.await
.unwrap();
} = process_execution::make_execute_request(&cat_roland_request(), None, None, &store, None)
.await
.unwrap();

mock::execution_server::TestServer::new(
mock::execution_server::MockExecution::new(vec![
Expand All @@ -1922,7 +1849,7 @@ async fn execute_missing_file_uploads_if_known() {
},
ExpectedAPICall::Execute {
execute_request: process_execution::make_execute_request(
&cat_roland_request().try_into().unwrap(),
&cat_roland_request(),
None,
None,
&store,
Expand Down
Loading

0 comments on commit d88e258

Please sign in to comment.