A pattern has emerged where a integration tests check for the
availability of an external tool (`git`, `taplo`, `gpg`, ...) and skip
the test (by simply passing it) when it is not available. To check this,
the program is run with the `--version` flag.
Some tests require that the program be available at least when running
in CI, by calling `ensure_running_outside_ci` conditionally on the
outcome. The decision is up to each test, though, the utility merely
returns a `bool`.
The previous implementation of `assert_no_forgotten_test_files`
hard-coded the name of the `runner` integration test and required all
other source files to appear in matching `mod` declarations. Thus, this
approach cannot handle multiple integration tests.
However, additional integration tests may be desirable
- to support tests using a custom test harness (see upcoming commits)
- to balance the trade-off between test run time and compile time as
the test suite grows in the future.
The new implementation first uses `taplo` to parse the `[[test]]`
sections of the manifest to identify integration test main modules,
and then searches in those for `mod` declarations. This is then compared
to the list of source files in the tests directory. Like the previous
implementation, the new one does not attempt to recurse into submodules
or to handle directory-style modules; just like before it only treats
source files without a module declaration as an error and relies on the
compiler to complain about the other way around.
When `taplo` is not installed, the check is skipped unless it is running
in CI where we require `taplo` to be available.
We ran into a crash on our server at Google today because we
accidentally called `RepoPathBuf::from_internal_string()` with a
string starting with a '/', which resulted in a the assertion in that
function failing. This patch changes that constructor and its siblings
to return a `Result` instead.
Fix GHSA-794x-2rpg-rfgr.
`gix::Repository::work_dir` was renamed to `workdir` (though strangely
not the `gix::ThreadSafeRepository` version), and `lossy_config`
is now off by default in all configurations.
Previously, this was calling `git_repository_open()`,
which is equivalent to `git_repository_open_ext()` with
`flags = GIT_REPOSITORY_OPEN_NO_SEARCH` and `ceiling_dirs =
NULL`. This changes `ceiling_dirs` to an empty string, and adds
`GIT_REPOSITORY_OPEN_FROM_ENV` to `flags` when we’re in test code.
`GIT_REPOSITORY_OPEN_FROM_ENV` is used to respect the Git configuration
path environment variables, which is what we want for the test
hermeticity code. It works like this:
* `config_path_system` will use `$GIT_CONFIG_SYSTEM` because `use_env`
will be set.
* `config_path_global` will use `$GIT_CONFIG_GLOBAL` because `use_env`
will be set.
* `git_config__find_xdg` and `git_config__find_programdata` will find
impure system paths and load them even when `$GIT_CONFIG_GLOBAL` is
set, contrary to Git behaviour, so we need to set `$XDG_CONFIG_HOME`
and `$PROGAMDATA`.
It has a few other effects, which I will exhaustively enumerate to
show that they are benign:
* It respects `$GIT_WORK_TREE` and `$GIT_COMMON_DIR`. These would
already break our tests, I think, so we’re assuming they’re
not set. (Possibly we should set them explicitly.)
* When opening a repository, it will:
* Set the starting path for the search to `$GIT_DIR` if it’s
`NULL`, but we do set it, so no change.
* Initialize `ceiling_dirs` to `$GIT_CEILING_DIRECTORIES` if it’s
`NULL`, but we do set it, so no change.
* Respect `$GIT_DISCOVERY_ACROSS_FILESYSTEM` and set the
`GIT_REPOSITORY_OPEN_CROSS_FS` flag appropriately. However,
this is only checked on subsequent iterations of the loop in
`find_repo_traverse`, and we set `GIT_REPOSITORY_OPEN_NO_SEARCH`
which causes it to never enter a second iteration.
* Use `ceiling_dirs` in `find_ceiling_dir_offset`, but the result is
ignored when `GIT_REPOSITORY_OPEN_NO_SEARCH` is set, so changing
from `NULL` to the empty string doesn’t affect behaviour. (It
also would always return the same result for either value, anyway.)
The gix feature "blocking-network-client" was configured in cd6141693
(cli/tests: add gitoxide helpers, 2025-02-03) most likely because
we needed to clone using gix in tests, but 071e724c1 (cli/tests:
move test_git_colocated_fetch_deleted_or_moved_bookmark to gitoxide,
2025-02-25) changed the test to clone by subprocessing out to system git
(not by using gitoxide, as a cursory read of the commit description may
indicate), meaning that we no longer need that feature.
Therefore, remove that feature.
These helpers are going to be needed to port the git2 code in the lib
tests to gitoxide. Since the cli tests already depend on testutils, this
helps with avoiding duplicating the code
We need to make `TestSigningBackend` available for cli tests, as we will
add cli tests for signing related functionality (templates for
displaying commit signatures, `jj sign`) in upcoming commits.
Co-authored-by: julienvincent <m@julienvincent.io>
It's nice that "jj config list --include-defaults" can show these default
values.
I just copied the jj-cli directory structure, but it's unlikely we'll add more
config/*.toml files.
While this is a debug option, it doesn't make sense to store an integer value
as a string. We can parse the environment variable instead. The variable is
temporarily parsed into i64 because i64 is the integer type of TOML.
toml_edit::Value doesn't implement any other integer conversion functions.
UserSettings::get_*() will be changed to look up a merged value from
StackedConfig, not from a merged config::Value. This will help migrate away
from the config crate.
Not all tests are ported to ConfigLayer::parse() because it seemed a bit odd
to format!() a TOML document and parse it to build a table of configuration
variables.
I'm going to add "checked" version of to_fs_path(), but all callers can't be
migrated to it. For example, an error message should be produced even if the
path is malformed.
This patch also adds error variants to propagate InvalidRepoPathError. They
don't use ::Other { .. } so the errors can be distinguished in tests.
This unblocks the use of TestBackend in long-running processes such as fuzzer.
It should also be safer because TempDir doesn't guarantee that the path is never
reused.
TestBackendData instances persist in memory right now, but they should be
discarded when the corresponding temp_dir gets dropped. The added struct will
manage the TestBackendData mapping.
