Skip to content

Commit

Permalink
Enable MockGet when using call-by-name (pantsbuild#21144)
Browse files Browse the repository at this point in the history
The scaffolding required to use `MockGet` as advertised required `Call`
exposing some variables that were kept internal compared to `Get`.

I think this a good compromise to keep the migration to call-by-name
smooth. Especially since plugins out in the wild might utilize `MockGet`
more than pants does internally as it is advertized in the
[docs](https://www.pantsbuild.org/2.22/docs/tutorials/testing-plugins#unit-testing-for-rules),
and plugin authors probably want their plugins to be fast, too.
  • Loading branch information
tobni authored Aug 3, 2024
1 parent 1ec5fea commit 9b625e4
Show file tree
Hide file tree
Showing 5 changed files with 62 additions and 14 deletions.
9 changes: 7 additions & 2 deletions src/python/pants/backend/javascript/subsystems/nodejs.py
Original file line number Diff line number Diff line change
Expand Up @@ -41,10 +41,11 @@
from pants.engine.env_vars import EnvironmentVars, EnvironmentVarsRequest, PathEnvironmentVariable
from pants.engine.fs import EMPTY_DIGEST, CreateDigest, Digest, Directory, DownloadFile
from pants.engine.internals.native_engine import FileDigest, MergeDigests
from pants.engine.internals.platform_rules import environment_vars_subset
from pants.engine.internals.selectors import MultiGet
from pants.engine.platform import Platform
from pants.engine.process import Process, ProcessResult
from pants.engine.rules import Get, Rule, collect_rules, rule
from pants.engine.rules import Get, Rule, collect_rules, implicitly, rule
from pants.engine.unions import UnionRule
from pants.option.option_types import DictOption, ShellStrListOption, StrListOption, StrOption
from pants.option.subsystem import Subsystem
Expand Down Expand Up @@ -390,7 +391,11 @@ class NodeJSBootstrap:
async def _get_nvm_root() -> str | None:
"""See https://github.com/nvm-sh/nvm#installing-and-updating."""

env = await Get(EnvironmentVars, EnvironmentVarsRequest(("NVM_DIR", "XDG_CONFIG_HOME", "HOME")))
env = await environment_vars_subset(
**implicitly(
{EnvironmentVarsRequest(("NVM_DIR", "XDG_CONFIG_HOME", "HOME")): EnvironmentVarsRequest}
)
)
nvm_dir = env.get("NVM_DIR")
default_dir = env.get("XDG_CONFIG_HOME", env.get("HOME"))
if nvm_dir:
Expand Down
4 changes: 4 additions & 0 deletions src/python/pants/engine/internals/native_engine.pyi
Original file line number Diff line number Diff line change
Expand Up @@ -897,6 +897,10 @@ _Output = TypeVar("_Output")
_Input = TypeVar("_Input")

class PyGeneratorResponseCall:
output_type: type
input_types: Sequence[type]
inputs: Sequence[Any]

@overload
def __init__(
self,
Expand Down
6 changes: 3 additions & 3 deletions src/python/pants/testutil/rule_runner.py
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@
from pants.engine.internals import native_engine
from pants.engine.internals.native_engine import ProcessExecutionEnvironment, PyExecutor
from pants.engine.internals.scheduler import ExecutionError, Scheduler, SchedulerSession
from pants.engine.internals.selectors import Effect, Get, Params
from pants.engine.internals.selectors import Call, Effect, Get, Params
from pants.engine.internals.session import SessionValues
from pants.engine.platform import Platform
from pants.engine.process import InteractiveProcess, InteractiveProcessResult
Expand Down Expand Up @@ -737,7 +737,7 @@ def run_rule_with_mocks(
if not isinstance(res, (Coroutine, Generator)):
return res

def get(res: Get | Effect):
def get(res: Get | Effect | Call):
provider = next(
(
mock_get.mock
Expand Down Expand Up @@ -767,7 +767,7 @@ def get(res: Get | Effect):
while True:
try:
res = rule_coroutine.send(rule_input)
if isinstance(res, (Get, Effect)):
if isinstance(res, (Get, Effect, Call)):
rule_input = get(res)
elif type(res) in (tuple, list):
rule_input = [get(g) for g in res] # type: ignore[union-attr]
Expand Down
53 changes: 44 additions & 9 deletions src/rust/engine/src/externs/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,6 @@
// File-specific allowances to silence internal warnings of `[pyclass]`.
#![allow(clippy::used_underscore_binding)]

use std::cell::RefCell;
use std::collections::BTreeMap;
use std::convert::TryInto;
use std::fmt;

use futures::future::{BoxFuture, Future};
use futures::FutureExt;
use lazy_static::lazy_static;
Expand All @@ -18,6 +13,11 @@ use pyo3::types::{PyBytes, PyDict, PySequence, PyTuple, PyType};
use pyo3::{create_exception, import_exception, intern};
use pyo3::{FromPyObject, ToPyObject};
use smallvec::{smallvec, SmallVec};
use std::cell::{Ref, RefCell};
use std::collections::BTreeMap;
use std::convert::TryInto;
use std::fmt;
use std::ops::Deref;

use logging::PythonLogLevel;
use rule_graph::RuleId;
Expand Down Expand Up @@ -511,6 +511,16 @@ impl PyGeneratorResponseNativeCall {
#[pyclass(subclass)]
pub struct PyGeneratorResponseCall(RefCell<Option<Call>>);

impl PyGeneratorResponseCall {
fn borrow_inner(&self) -> PyResult<impl Deref<Target = Call> + '_> {
Ref::filter_map(self.0.borrow(), |inner| inner.as_ref()).map_err(|_| {
PyException::new_err(
"A `Call` may not be consumed after being provided to the @rule engine.",
)
})
}
}

#[pymethods]
impl PyGeneratorResponseCall {
#[new]
Expand Down Expand Up @@ -544,6 +554,34 @@ impl PyGeneratorResponseCall {
inputs,
}))))
}

#[getter]
fn output_type<'p>(&'p self, py: Python<'p>) -> PyResult<&'p PyType> {
Ok(self.borrow_inner()?.output_type.as_py_type(py))
}

#[getter]
fn input_types<'p>(&'p self, py: Python<'p>) -> PyResult<Vec<&'p PyType>> {
Ok(self
.borrow_inner()?
.input_types
.iter()
.map(|t| t.as_py_type(py))
.collect())
}

#[getter]
fn inputs<'p>(&'p self, py: Python<'p>) -> PyResult<Vec<PyObject>> {
let inner = self.borrow_inner()?;
let args: Vec<PyObject> = inner.args.as_ref().map_or_else(
|| Ok(Vec::default()),
|args| args.to_py_object().extract(py),
)?;
Ok(args
.into_iter()
.chain(inner.inputs.iter().map(Key::to_py_object))
.collect())
}
}

impl PyGeneratorResponseCall {
Expand Down Expand Up @@ -640,10 +678,7 @@ impl PyGeneratorResponseGet {
})?
.inputs
.iter()
.map(|k| {
let pyo: PyObject = k.value.clone().into();
pyo
})
.map(Key::to_py_object)
.collect())
}

Expand Down
4 changes: 4 additions & 0 deletions src/rust/engine/src/python.rs
Original file line number Diff line number Diff line change
Expand Up @@ -279,6 +279,10 @@ impl Key {
pub fn to_value(&self) -> Value {
self.value.clone()
}

pub fn to_py_object(&self) -> PyObject {
self.to_value().into()
}
}

// NB: Although `PyObject` (aka `Py<PyAny>`) directly implements `Clone`, it's ~4% faster to wrap
Expand Down

0 comments on commit 9b625e4

Please sign in to comment.