If you don't have toolchain installed and you use rustup run:
$ rustup toolchain install --profile default --component rustfmt,clippy 1.90
NOTE: It might be that you have 1.90.0 installed as stable toolchain, in
that case you still have to run the above command to install exactly 1.90.
Command: `just upgrade-rust`
Changes:
- Upgrade version of used toolchain in the following places:
- .gitlab-ci.yml
- Cargo.toml
- clippy.toml
- rust-toolchain.toml
Functionality:
User faced functionality is available through `just upgrade-rust`
recipe. It updates Rust versions in the following files through out the
project:
- .gitlab-ci.yml
- Cargo.toml
- clippy.toml
- rust-toolchain.toml
- Checks whether there are any changes of the working copy or in git
index and refuse to proceed if any asking to commit or stash them.
- Checks current git branch if it does not contains mentions of current stable
Rust version (with underscores instead of dots) it interactively propose to
create a new branch.
- Pulls current stable Rust version from
https://static.rust-lang.org/dist/channel-rust-stable.toml to upgrade
to it if not specific version was given, i.e. `just upgrade-rust 1.90`
upgrades version to 1.90 no metter what the current stable Rust
version is at the moment.
- Upgrades each of the place with where Rust version is used to pulled
current stable Rust version outputing details about upgrade:
- .gitlab-ci.yml
- Cargo.toml
- clippy.toml
- rust-toolchain.toml
- Interactively asks whether to commit the changes and if agreed commits them
to git with detailed message of what was upgraded and to which version.
- Reminds about the need to fix possible compilation and linting errors and
warnings after upgrade.
Implementation:
Functionality is delivered through main public "upgrade-rust" just
recipe and set of private (starts with underscore) recipes. Private
recipes still can be called from command line
(i.e. for debugging purposes) but they are not listed in the list of
tasks when you type `just` or `just --list`.
For example, you can still call `just _rust-stable-version` to see what
is current stable version of Rust which is retrieved by script.
"upgrade-rust" has dependency (or pre-dependency)
"_ensure-no-vcs-changes" recipe to check for absence of code changes and
post-dependency "_upgrade-rust-fixes-reminder" for the reminder of
compilation and linting fixes.
The main body of "upgrade-rust" recipe extract current versions of Rust
from number of files (called OLD in the code) to be able to output
upgrade messages of what was upgraded.
For this it uses the following private recipes:
- _current-ci-rust-version
- _current-cargo-rust-version
- _current-clippy-rust-version
- _current-toolchain-rust-version
Each of that recipes (with help of variables) encodes specifics of what, how
and from which file should be retrieved.
"_upgrade-rust-git-create-branch" makes check for the current git branch
and interactively propose to create new one switching to it.
The main workflow of version upgrade in different files is in
"_upgrade-rust" private recipe
(yep, there is upgrade-rust user faced recipe and _upgrade-rust a private one).
"_upgrade-rust" is supplied with specific values to upgrade Rust version
e.g. in .gitlab-ci.yml or in rust-toolchain.toml.
So, "upgrade-rust" calls the following tasks
- _upgrade-rust-ci
- _upgrade-rust-cargo
- _upgrade-rust-clippy
- _upgrade-rust-toolchain
to do the upgrade which supply details to "_upgrade-rust" and call it.
After changes of version "upgrade-rust" calls "_upgrade-rust-git-commit"
recipe to commit changes supplying all the details about upgrade, e.g.
old versions, new version and list of files where Rust version was
changed. "_upgrade-rust-git-commit" asks whether you want to commit
changes, if you agree it makes up a commit message and produce commit.
Functionality of editing quite heavily rely on "sed" utility with some
help from tq (tomlq).
For VCS related operations like branch creation, commiting "git" CLI is
used.
Extension:
The implementation design makes it fairly easy to extend recipes to
support new places where Rust has to be upgraded or remove such support
if some files was removed from source tree.
To add support for new place to perform upgrade:
- Add bunch of variables to justfile, e.g.
CARGO_FILE_NAME := 'Cargo.toml'
CARGO_FILE_PATH := absolute_path(CARGO_FILE_NAME)
CARGO_RUST_VERSION_QUERY := 'package.rust-version'
Addition of CARGO_RUST_VERSION_QUERY depends on format of your file
and how you will implement your "_current-cargo-rust-version".
Obviously replace CARGO/cargo in names of variables and recipes with
your place name.
- Add "_current-cargo-rust-version" recipe to retrieve currently used
Rust version from your location, rename the function according to the
new place.
- Add place specific upgrade recipe named properly, .e.g "_upgrade-rust-cargo"
which calls to "_upgrade-rust" and supply all the required arguments like
file path, file name, target version, sed command to change the
version and some parts of the messages to make proper reporting about
upgrade process.
- Add newly added recipes to the for-loops in "upgrade-rust" recipe, so
they can be called.
- Add newly added FILE_PATH and FILE_NAME variables to the call of
"_upgrade-rust-git-commit" in "upgrade-rust" recipe, so new place can
be listed in git commit and the changes in it can be added to the
commit.
Changes:
- Extend list of dependencies in README.md with command line tools
required for "upgrade-rust" recipe:
- curl
- sed
- tq (from `tomlq` crate)
- git
- Refer to "upgrade-rust" recipe in newly added "Contribution/Housekeeping"
sesion of README.md.
- Add bunch of variables to justfile related to number of private
recipes to compound functionality of "upgrade-rust":
- SED_RUST_VERSION_REGEX
- RUST_MANIFEST_STABLE_TOML_URL
- RUST_MANIFEST_STABLE_VERSION_QUERY
- RUST_MANIFEST_STABLE_VERSION_PARSE_REGEX
- DEFAULT_RUST_STABLE_VERSION_FORMAT
- GITLAB_CI_FILE_NAME
- GITLAB_CI_FILE_PATH
- CARGO_FILE_NAME
- CARGO_FILE_PATH
- CARGO_RUST_VERSION_QUERY
- CLIPPY_FILE_NAME
- CLIPPY_FILE_PATH
- CLIPPY_RUST_VERSION_QUERY
- TOOLCHAIN_FILE_NAME
- TOOLCHAIN_FILE_PATH
- TOOLCHAIN_RUST_VERSION_QUERY
- GIT_BRANCH_NAME_PREFIX
- Add "upgrade-rust" recipe which upgrade Rust version in the following
files:
- .gitlab-ci.yml
- Cargo.toml
- clippy.toml
- rust-toolchain.toml
- Add private just recipes to delivery different aspects of compound
functionality of "upgrade-rust" recipe:
- _ensure-no-vcs-changes
- _upgrade-rust-git-create-branch
- _upgrade-rust-fixes-reminder
- _upgrade-rust-git-commit
- _upgrade-rust
- _upgrade-rust-ci
- _upgrade-rust-cargo
- _upgrade-rust-clippy
- _upgrade-rust-toolchain
- _rust-stable-version
- _current-ci-image
- _current-ci-rust-version
- _current-cargo-rust-version
- _current-clippy-rust-version
- _current-toolchain-rust-version
Changes:
- Split hagrid::web::tests::maintenance() tests to the following:
- hagrid::routes::vks:tests::get_upload:::maintenance()
- hagrid::routes::manage::tests::get_manage::maintenance()
- hagrid::routes::vks:tests::get_verify_token:::maintenance()
- hagrid::routes::pks::tests::post_pks_add_multipart_form_data::maintenance()
- hagrid::routes::api::rest::vks::tests::post_vks_v1_upload::maintenance()
- hagrid::routes::api::rest::vks::tests::post_vks_v1_request_verify_json::maintenance()
- hagrid::routes::vks::tests::put_root::maintenance()
- Fix tests' endpoints to make them more accurate and actually match
existing endpoints:
- GET /verify -> GET /verify/<token>
- GET /pks/add -> POST /pks/add/ [Content-Type: multipart/form-data]
- GET /vks/v1/upload -> POST /vks/v1/upload
- GET /vks/v1/request-verify -> POST /vks/v1/request-verify
- Verify that maintenance state has dissapeared after maintenance file
removal in each individual test with the same endpoint on which we
asserted maintenance mesage.
Previous tests checked only maintenance message disappearance on GET /upload,
which wasn't completely correct.
- Introduce new fixtures at hagrid::routes::tests::common:
- token
- serialized_cert
- key_multipart_form_data
- Introduce new test module: hagrid::routes::tests::common::test.
The module is going to accumulate common tests which define common
workflow for the test and can be called from other tests to check for
specific behaviour of concreate function or HTTP endpoint.
- Generalize functionality of maintenance test. Move it to module with
shared tests: hagrid::routes::tests::common::test::maintenance().
Changes:
- Extract and generalize assert for absence of the maintenance text from
hagrid::web::tests::maintenance() test to
hagrid::routes::tests::common::assert::response_does_not_contain_text()
assert helper function. Reuse it in the test instead.
- Refactor hagrid::web::tests::common::assert::check_maintenance()
function to make it more reusable and remove code duplication in its
implementation:
- Instead of duplicating response asserting code call for the helper
hagrid::routes::tests::common::assert::response() and prefill the
status.
- Make it accept response instead of passing in client and uri. This
make check_maintenance() helper more reusable as now we are not
dependent on what HTTP verb is used to get the response. In
addition, request occurs directly inside body of the test and helper
just assert the response. Hence, reparation of responsibilities and
following SRP.
- Let check_maintenance() accept the maintenance text which is checked
in response. The previous implementation of the
hagrid::web::tests::maintenance() test and the helper was frigile as
it allows a lot of possibilities for maintenance text to go out of
coherence.
- Adjust code in hagrid::web::tests::maintenance() test according to
changed signature of check_maintenance() assertion helper.
- Remove hardcoded checks in hagrid::web::tests::maintenance() test and
reuse assertion helpers.
Changes:
- Move tests of hagrid::web::tests::check_response() for /search
endpoint to hagrid::routes::vks::tests::get_search::not_supported_search_query().
Refactor tests along the way to get rid of boilerplate code.
- Move test of hagrid::web::tests::check_response() for /pks/lookup
endpoint to hagrid::routes::pks::tests::get_pks_lookup::not_supported_search_query().
Refactor tests along the way to get rid of boilerplate code.
- Move test of hagrid::web::tests::check_response() for
/.well-known/openpgpkey/<domain>/<policy> endpoint to
hagrid::routes::wkd::tests::get_wkd_policy::wkd_policy_respond_successfully_with_empty_body().
Refactor tests along the way to get rid of boilerplate code.
Changes:
- Replace usage of hagrid::web::tests::common::assert::check_response()
with hagrid::routes::tests::common::assert::response() to remove code
duplication in tests.
- Refactor hagrid::web::tests::check_response() tests to use
assert::response() helper.
- Take &str instead of &'static str as present_page_text argument in
assert::response() helper. This was a bug. It affected reusability of
the assertion helper function.
"rocket" fixture ATM used only by "client" fixture. So, it can be
internal to the hagrid::routes::tests::common module. Its export caused
clash of names with "rocket" module from "rocket" crate which forced to
prefix module from crate with ::, i.e. ::rocket.
Using absolute path to the module is probably a good thing by itself
when it used consistently across code base, but in Hagrid's code base it
wasn't and being forced to do that by that tricky coincidence was
annoying and confusing.
There several caveats to the refactoring:
- Before refactoring all tests ran in strong order as they were
sequential code in a single function. I believe this was an accident
behaviour and there were nothing in functionality of basics() fn that
forced certain sequence of test execution. So, using table testing
with random order considered by me fine.
- basics() fn tests only read queries. So, it was a discovery for me to
find at the end of the function check for consistency,
(i.e. assert_consistency()) as it logically does not make sense.
I believe that was a test code copy-paste mistake.
To preserve basic check I moved it to dedicated test basic_consistency().
Fixtures allow for push-style cascading dependency resolution with
ability to override some of the dependent fixture's arguments when needed.
This is very nice as it gives a lot of flexibility and suitable in a lot
of case. So, single fixture based approach can be applied to most of the
tests.
In addition, this result in good reusability of test code.
If you feel overwelmed and confused with complex argument list of test
functions try to read links from "Documentation" section (see below).
But here is the primer.
#[fixture]
pub fn configuration(
base_uri: &str,
base_uri_onion: &str,
) -> (TempDir, :🚀:figment::Figment) {
// ...
}
Attribute #[fixture] turn function into fixture which means now it can
be injected into tests or other fixtures by "configuration" argument name.
"base_uri", "base_uri_onion" are other fixtures which are injected into
"configuration".
#[fixture]
pub fn rocket(
#[from(configuration)] (tmpdir, config): (TempDir, :🚀:figment::Figment),
) -> (TempDir, :🚀:Rocket<:🚀:Build>) {
// ...
}
As we destructuring results of "configuration" fixture injection into
"rocket" fixture the name of the fixture can't be inferenced from
parameters name, so we have to give a hint with #[from(configuration)]
which fixture has to be called.
#[rstest]
fn hkp_add_two(
base_uri: &str,
#[from(cert)] (_, tpk_0): (&str, Cert),
#[with(alt_cert_name())]
#[from(cert)]
(_, tpk_1): (&str, Cert),
#[from(client)] (tmpdir, client): (TempDir, Client),
) {
// ...
}
This is probably the most trickies function signature, but let brake it
down.
There are 4 fixtures injected into "hkp_add_two" test function:
- base_uri
- cert
- cert
- client
I think at this point of explanation syntax for "base_uri" and "client"
fixturex should be clear.
So,
#[from(cert)] (_, tpk_0): (&str, Cert),
#[with(alt_cert_name())] #[from(cert)] (_, tpk_1): (&str, Cert),
calls to "cert" fixture which takes "cert_name" argument. Here is the
definition:
#[fixture]
pub fn cert<'a>(cert_name: &'a str) -> (&'a str, Cert) {
// ...
}
For the "hkp_add_two" test we need two certificates, so first one is
generated with default name which is returned by "cert_name" fixture,
but for the second with #[with(alt_cert_name())] we say that we want to
use "alt_cert_name" fixture for cert_name argument, so the certificate
is generated with "bar@invalid.example.com" name.
And parenthis for destructing of the results of injection as "cert"
fixture returns tuple of (cert_name, cert).
Documentation:
- https://crates.io/crates/rstest
- https://docs.rs/rstest/0.26.1/rstest/attr.fixture.html
NOTE: probably this changes made translation generation in tests more unstable.
It was already unstable, but it looks like the use of fixtures make a
bit more unstable. So, in case of test failures, just run them several
times. I'll take a look and modules compilation order later once move
tests into modules with SUT's functionality.
Changes:
- Turn the following helper functions from hagrid::web::tests
module to fixtures:
- configuration()
- rocket()
- client()
- Panic immediately in the fixtures and don't bubble up the error like
it was in helper functions. Bubbling up errors does not make sense as
we panic with expect later in tests code. So, we duplicated code
(.unwrap()) calls) and make return signatures more complex.
- Inject "base_uri" and "base_uri_onion" into "configuration" fixture as
after turning it into fixture we can do that.
- Remove BASE_URI_ONION constant as it is not used. Now "base_uri_onion"
fixture is used instead.
- Refactor all the tests which used mentioned above helper functions to
use fixture.
Changes:
- Turn "build_cert" funtion into fixture.
- Refactor all tests which relied on build_cert() to inject one or two
certs.
- Replace usage of duplicated e-mail (cert_name) in tests with value
returned by "cert" fixture (i.e. cert_name). This better align actual
cert and cert_name usage in tests and eliminate hardcoded e-mail
string slices.
Changes:
- Create fixtures based on constants:
- base_uri
- base_uri_onion
- Change tests which use BASE_URI or BASE_URI_ONION from #[test]
declaration to #[rstest]. That allows to inject fixtures.
- Replace usage of constants BASE_URI and BASE_URI_ONION with base_uri
and base_uri_onion fixtures.
There are still some usages of the constants which will be refactored
later.
- Propagate injected fixture values into check_* assertions to replace
constant usages.
Command: `just check`
Changes:
- The following warnings were fixed:
warning: hiding a lifetime that's elided elsewhere is confusing
--> database/src/sqlite.rs:146:11
|
146 | fn tx(&self) -> &Transaction {
| ^^^^^ ------------
| | ||
| | |the same lifetime is hidden here
| | the same lifetime is elided here
| the lifetime is elided here
|
= help: the same lifetime is referred to in inconsistent ways, making the signature confusing
= note: `#[warn(mismatched_lifetime_syntaxes)]` on by default
help: use `'_` for type paths
|
146 | fn tx(&self) -> &Transaction<'_> {
| ++++
warning: hiding a lifetime that's elided elsewhere is confusing
--> src/dump.rs:22:30
|
22 | pub fn display_sensitive(&self) -> SessionKeyDisplay {
| ^^^^^ ----------------- the same lifetime is hidden here
| |
| the lifetime is elided here
|
= help: the same lifetime is referred to in inconsistent ways, making the signature confusing
= note: `#[warn(mismatched_lifetime_syntaxes)]` on by default
help: use `'_` for type paths
|
22 | pub fn display_sensitive(&self) -> SessionKeyDisplay<'_> {
| ++++
If you don't have toolchain installed and you use rustup run:
$ rustup install --component rustfmt --component clippy 1.89
NOTE: It might be that you have 1.89.0 installed as stable toolchain, in
that case you still have to run the above command to install exactly
1.89.
Changes:
- Upgrade version of used toolchain in the following places:
- .gitlab-ci.yml
- Cargo.toml
- clippy.toml
- rust-toolchain.toml
Continue to dissolve hagrid::web module which had way too much responsibilities
and became the god-module (see https://en.wikipedia.org/wiki/God_object).
In addition, solved and refined several smaller issues and made
improvements.
Changes:
- Extract configuration loading (i.e. extraction) from
hagrid::web::rocket_factory() to hagrid::config::load() and call it
from hagrid::app::configure_rocket().
- Extract and split hagrid::web::run() (which previously was #[launch] on
rocket() fn) to hagrid::app::run(), hagrid::app::configure_rocket()
and hagrid::app::run_rocket().
- Remove String copy in Configuration::base_uri_onion() and return
references to internal fields of the struct. Callers can copy it when
they need to, otherwise they can just use references without overhead.
- Extract rocket() helper function from client() in tests of hagrid::web
module to separate rocket instance creation.
- Remove code duplication in tests of hagrid::web module by reusing
rocket() and client() functions for instances construction.
- Introduce idea of initializers and "hagrid::initializers" module to bootstrap
rocket framework and register states (with rocket.manage()), fairings (with
rocket.attach()) and routes (with rocket.mount()) on rocket instance.
A single initializer resides in submodule of hagrid::initializers and
register itself in hagrid::initializers::run() function (has to be
done manually by the programmer who adds new initalizer).
Initializer consist of the following two functions with some vary signatures:
- init(config: &Configuration) -> hagrid::initalizers::Return<CONSTRUCTED_TYPE>
- register(rocket: Rocket<Build>, config: &Configuration, state_fairing_or_routes: <CONSTRUCTED_TYPE>) -> Rocket<Build>
init(config: &Configuration)
The function instantiate state, fairing or routes possibly using
config for it.
Tricky moment about it is its return type which has to be
hagrid::initializers::Result. This return type is an alias to
anyhow::Result<Option<T>>. When construction fails for whatever reason it
returns Err. If everything went well, but construction is not a desired
behaviour, i.e. because of it was disabled in configuration, like we
do for "prometheus" initializer init() should return Ok(None).
Errors then handled in hagrid::initializers::run() function (by
initialize!() macro to be precise) and register the instance if it was
created.
register(rocket: Rocket<Build>, config: &Configuration, state_fairing_or_routes: <CONSTRUCTED_TYPE>)
Receives unwrapped instance of state, fairing or routes if it was
constructed and register it on rocket.
Some instances can be registered several times, e.g. like as fairing
and as routes. We do that for PrometheusMetrics.
- Move hagrid::web::ApplicationState (what was HagridState before
renaming) to hagrid::app::state::ApplicationState.
- Dissolve hagrid::web::rocket_factory() and set of configure_*
functions into initializers preserving the order of their registration
on rocket, though it might be not so important.
- Replace usage of hagrid::web::rocket_factory() in hagrid::web
module's tests with hagrid::app::configure_rocket() which become
approximate functional equivalent of it but calls for initializers to
populate the rocket.
