forked from GitoxideLabs/gitoxide
-
Notifications
You must be signed in to change notification settings - Fork 0
Commit f360666
committed
Let
`gix-testtools` depends on several other `gix-*` crates. Before
version 0.16.0 (GitoxideLabs#1972), `gix-testtools` depended on prior breaking
versions of those crates (as discussed in GitoxideLabs#1510 and GitoxideLabs#1886). Since
then, it depends on the current versions.
When depending on a strictly earlier version, it was necessary to
omit `path =` in the `gix-testtools` manifest for its `gix-*`
dependencies. Now that `gix-testtools` depends on current versions
of those dependencies, it seems feasible to specify `version` and
`path`, as we do in other cases where one crate developed in this
workspace depends on another crate developed in this workspace.
Aside from improving general consistency (which is a weak rationale
here, since the role of `gix-testools` differs substantially from
that of other `gix-*` crates, in terms of how we're ourselves using
it), the benefit here is that ambiguity in what crate is meant, when
an operation is performed on a specific `gix-*` crate, is lessened,
or maybe even eliminated.
In particular, a number of actions we prefer `<cmd> -p <crate>` for
were done by `(cd <crate-dir>; <cmd>)` to operate on `gix-*` crates
in the workspace that are also dependencies, even transitively, of
`gix-testtools`. This affected some commands in `justfile` recipes,
some commands run in CI workflows (indirectly via `just`, or
directly in script steps), some some operations carried out
manually. This included `cargo nextest run` and `cargo check` on
various crates.
Here's an example (shown on Windows, but this problem was not
specific to Windows) using `gix-date`, which is not listed in
`tests/tools/Cargo.toml`, but which is a transitive dependency:
C:\Users\ek\source\repos\gitoxide [main ≡]> cargo nextest run -p gix-date
Blocking waiting for file lock on package cache
error: There are multiple `gix-date` packages in your project, and the specification `gix-date` is ambiguous.
Please re-run this command with one of the following specifications:
path+file:///C:/Users/ek/source/repos/gitoxide/gix-date#0.10.1
registry+https://github.com/rust-lang/crates.io-index#[email protected]
error: command `'\\?\C:\Users\ek\.rustup\toolchains\stable-x86_64-pc-windows-msvc\bin\cargo.exe' test --no-run --message-format json-render-diagnostics --package gix-date` exited with code 101
An important special case is that of editor/IDE integration, such
as in VS Code. This couldn't run and (more significantly, in view
of the benefit of integration) couldn't debug some of the tests.
This happened because synthesized `cargo test -p ...` commands,
used behind the scenes to launch the tests, were ambiguous.
A further benefit is that the lockfile and dependency tree are
simpler.
However, that points to an important aspect of this change: it is
more than a refactoring. Although it shouldn't produce different
behavior when `gix-testtools` is obtained from crates.io (i.e. when
projects developed outside the `gitoxide` repository use
`gix-testtools`), it can produce different behavior here, where
`gix-testtools` will use changes to its `gix-*` dependencies (and
accordingly their own dependencies, recursively) that are present
in the workspace even if not present in the released version that
matches `version =`.
That could be a good thing if it causes new changes to be exercised
more and earlier. That might help find bugs. But:
- It could be bad if it introduces an undesirable dependency
ordering for fixing bugs and/or introducing regression tests.
That is, in principle there could arise two (possibly related)
bugs, A and B, where there is some reason to fix A before B, but
where B must be fixed in order for the regression test for A to
run (to validate that it can catch A), due to B breaking
`gix-testtools` as used in the test for A or in other tests in
the crate affected by A.
Because this would presumably be known--an error would occur,
likely when building the tests--it could be worked around by
temporarily (or permanently) reverting this change if and when
such a problem ever arises, or partially undoing it for the
specific affected `gix-*` dependency of `gix-testtools`.
- It could be bad if a bug affects a `gix-*` crate and its own
tests in identical or complementary ways, and this is used to
establish or check an expectation.
That is, in principle there could arise a bug in a `gix-*` crate
that `gix-testtools` uses, and that itself uses `gix-testtools`
in its tests, that causes a test that should catch that bug
(either initially or to verify a bugfix) to wrongly report that
the code is working.
This scenario is a case of the general problem that duplicated
logic between code and its tests can cause a bug to appear
(either in the same form or in different forms) in both, such
that tests that should catch the bug don't catch it because they
suffer from the same bug. In the hypothetical case imagined here,
the duplication of logic would arise from the tests calling and
using the very code that is under test.
For the way we are currently using or likely ever to use
`gix-testtools`, it seems like this would probably not happen.
But it is hard to be completely sure. Unlike the previously
described scenario, if this scenario did occur, it would likely
not be noticed.
Both those scenarios have corresponding scenarios that had already
applied (and which the change here at least slightly *mitigates*):
if the code with the bug has already been published.gix-testtools
use gix-*
workspace crates1 parent 83e1b73 commit f360666Copy full SHA for f360666
2 files changed
+333
-710
lines changed
0 commit comments