Skip to content

Commit

Permalink
ci: move a chunk of the Rust CI over to GHA (#9290)
Browse files Browse the repository at this point in the history
GHA has many significant benefits compared to buildkite, most major of
it is that blindly using GHA is not going to by default allow untrusted
contributors to run arbitrary code – instead GitHub will present
reviewers a button they can click after reviewing the PR.

It also makes maintenance of the CI infrastructure somewhat easier,
which given our soon-to-be-stretched-very-thin infrastructure team is a
huge benefit.

In process of implementing this PR I ended up simplifying a lot of the
Rust testing as well. In particular instead of running half a dozen of
different combinations on just Linux, we now run just the nightly vs
non-nightly versions. This saves CPU time on many different rebuilds… Of
note, one feature that no longer gets tested is `mock_node`, as it was
causing a failure in one of the integration tests. If we wanted to
re-enable this particular test, we should figure out how to fix the
test, rather than adding a new test configuration.

As a final benefit, I’ve also added a m1 macOS-based job. This should
help with making sure that people who develop on company-issued laptops
can actually be productive, rather than have to tip-toe around a
boatload of failing tests. We will have an ability to decide whether we
want to block PRs landing on this job in the repository configuration at
any point in the future.
  • Loading branch information
nagisa authored and nikurt committed Oct 11, 2023
1 parent 889fd84 commit 7a15a73
Show file tree
Hide file tree
Showing 13 changed files with 86 additions and 102 deletions.
59 changes: 1 addition & 58 deletions .buildkite/pipeline.yml
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
env:
RUST_BACKTRACE: short
RUSTFLAGS: -D warnings

steps:
- label: "protobuf backward compatibility"
Expand All @@ -15,63 +14,9 @@ steps:
- "distro=amazonlinux"
- "queue=default"

- label: "cargo test integration-tests*"
command: |
source ~/.cargo/env && set -eux
cargo install cargo-nextest
cargo nextest run --locked -p 'integration-tests'
agents:
- "distro=amazonlinux"
- "queue=default"
branches: "!master"

- label: "cargo test not integration-tests*"
command: |
source ~/.cargo/env && set -eux
cargo install cargo-nextest
cargo nextest run --locked --workspace -p '*' --exclude 'integration-tests*'
agents:
- "distro=amazonlinux"
- "queue=default"
branches: "!master"

- label: "cargo test nightly integration-tests*"
command: |
source ~/.cargo/env && set -eux
cargo install cargo-nextest
cargo nextest run --features nightly,test_features -p 'integration-tests'
agents:
- "distro=amazonlinux"
- "queue=default"
branches: "!master"

- label: "cargo test nightly not integration-tests*"
command: |
source ~/.cargo/env && set -eux
cargo install cargo-nextest
cargo nextest run --workspace --features nightly,test_features,mock_node -p '*' --exclude 'integration-tests*'
cargo test --doc
agents:
- "distro=amazonlinux"
- "queue=default"
branches: "!master"

- label: "sanity checks"
command: |
source ~/.cargo/env && set -eux
rustc --version && cargo --version
cargo install cargo-deny
cargo run -p themis --release
if [ -e deny.toml ]; then
cargo-deny --all-features check bans
fi
cargo check --workspace --all-targets --all-features
cargo check -p neard --features test_features
cargo check -p neard --features sandbox
cargo build -p neard --bin neard --features nightly
cd pytest
Expand All @@ -93,9 +38,7 @@ steps:
./scripts/formatting --check
./scripts/run_clippy.sh
rm target/rpc_errors_schema.json
rm -f target/rpc_errors_schema.json
cargo check -p near-jsonrpc --features dump_errors_schema
if ! git --no-pager diff --no-index chain/jsonrpc/res/rpc_errors_schema.json target/rpc_errors_schema.json; then
set +x
Expand Down
11 changes: 6 additions & 5 deletions .config/nextest.toml
Original file line number Diff line number Diff line change
@@ -1,17 +1,16 @@
[profile.default]
slow-timeout = { period = "60s", terminate-after = 2, grace-period = "0s" }
# FIXME(nagisa): use --profile ci in CI instead when we manage to modify CI scripts...
retries = { backoff = "fixed", count = 3, delay = "1s" }
failure-output = "final"

[[profile.default.overrides]]
filter = 'test(test_full_estimator)'
slow-timeout = { period = "10m", terminate-after = 3 }
threads-required = 4
retries = 0
threads-required = 2

[[profile.default.overrides]]
filter = 'package(style-tests)'
slow-timeout = { period = "120s", terminate-after = 5 }
retries = 0
threads-required = 4

# Unfortunately no support for inheriting profiles yet:
Expand All @@ -27,9 +26,11 @@ fail-fast = false
[[profile.ci.overrides]]
filter = 'test(test_full_estimator)'
slow-timeout = { period = "10m", terminate-after = 3 }
threads-required = 4
retries = 0
threads-required = 2

[[profile.ci.overrides]]
filter = 'package(style-tests)'
slow-timeout = { period = "120s", terminate-after = 5 }
retries = 0
threads-required = 4
42 changes: 42 additions & 0 deletions .github/workflows/rust.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
concurrency:
group: ${{ github.workflow }}-${{ github.ref }}
cancel-in-progress: true

on:
push:
branches: [master]
pull_request:
merge_group:

jobs:
cargo_nextest:
runs-on: ${{ matrix.os }}
strategy:
fail-fast: false
matrix:
include:
- os: ubuntu-22.04-8core
flags: ""
- os: ubuntu-22.04-8core
flags: "--features nightly,test_features"
- os: macOS-m1
# FIXME: some of these tests don't work very well on MacOS at the moment. Should fix
# them at earliest convenience :)
flags: "--exclude integration-tests --exclude node-runtime --exclude runtime-params-estimator --exclude near-network --exclude estimator-warehouse"
timeout-minutes: 90

steps:
# Some of the tests allocate really sparse maps, so heuristic-based overcommit limits are not
# appropriate here.
# FIXME(#9634): remove this once the issue is resolved.
- run: sudo sysctl vm.overcommit_memory=1 || true
- uses: actions/checkout@v2
- uses: baptiste0928/cargo-install@21a18ba3bf4a184d1804e8b759930d3471b1c941
with:
crate: cargo-nextest
- uses: baptiste0928/cargo-install@21a18ba3bf4a184d1804e8b759930d3471b1c941
with:
crate: cargo-deny
- run: cargo nextest run --locked --workspace -p '*' --cargo-profile quick-release --profile ci ${{ matrix.flags }}
env:
RUST_BACKTRACE: short
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion chain/jsonrpc/build_errors_schema.sh
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
#!/usr/bin/env bash
cd "${0%/*}/../.." # ensure we're in the workspace directory
rm target/rpc_errors_schema.json
rm -f target/rpc_errors_schema.json
cargo build -p near-jsonrpc --features dump_errors_schema
cp target/rpc_errors_schema.json chain/jsonrpc/res/rpc_errors_schema.json
4 changes: 3 additions & 1 deletion core/primitives/src/runtime/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -39,9 +39,10 @@ impl RuntimeConfig {

pub fn test() -> Self {
let config_store = super::config_store::RuntimeConfigStore::new(None);
let wasm_config = near_vm_runner::logic::Config::clone(
let mut wasm_config = near_vm_runner::logic::Config::clone(
&config_store.get_config(PROTOCOL_VERSION).wasm_config,
);
wasm_config.vm_kind = wasm_config.vm_kind.normalize();
RuntimeConfig {
fees: RuntimeFeesConfig::test(),
wasm_config,
Expand All @@ -54,6 +55,7 @@ impl RuntimeConfig {
let mut wasm_config = near_vm_runner::logic::Config::clone(
&config_store.get_config(PROTOCOL_VERSION).wasm_config,
);
wasm_config.vm_kind = wasm_config.vm_kind.normalize();
wasm_config.make_free();
Self {
fees: RuntimeFeesConfig::free(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ source: core/primitives/src/views.rs
expression: "&view"
---
{
"storage_amount_per_byte": "90900000000000000000",
"storage_amount_per_byte": "10000000000000000000",
"transaction_costs": {
"action_receipt_creation_config": {
"send_sir": 108059500000,
Expand All @@ -12,14 +12,14 @@ expression: "&view"
},
"data_receipt_creation_config": {
"base_cost": {
"send_sir": 4697339419375,
"send_not_sir": 4697339419375,
"execution": 4697339419375
"send_sir": 36486732312,
"send_not_sir": 36486732312,
"execution": 36486732312
},
"cost_per_byte": {
"send_sir": 59357464,
"send_not_sir": 59357464,
"execution": 59357464
"send_sir": 17212011,
"send_not_sir": 17212011,
"execution": 17212011
}
},
"action_creation_config": {
Expand All @@ -36,7 +36,7 @@ expression: "&view"
"deploy_contract_cost_per_byte": {
"send_sir": 6812999,
"send_not_sir": 6812999,
"execution": 6812999
"execution": 64572944
},
"function_call_cost": {
"send_sir": 2319861500000,
Expand Down Expand Up @@ -211,7 +211,7 @@ expression: "&view"
}
},
"account_creation_config": {
"min_allowed_top_level_account_length": 0,
"min_allowed_top_level_account_length": 32,
"registrar_account_id": "registrar"
}
}
20 changes: 9 additions & 11 deletions core/primitives/src/views.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2913,31 +2913,29 @@ impl From<ExtCostsConfigView> for near_primitives_core::config::ExtCostsConfig {

#[cfg(test)]
mod tests {
#[cfg(not(feature = "nightly"))]
use super::ExecutionMetadataView;
#[cfg(not(feature = "nightly"))]
use crate::transaction::ExecutionMetadata;
#[cfg(not(feature = "nightly"))]
use near_vm_runner::{ProfileDataV2, ProfileDataV3};

/// The JSON representation used in RPC responses must not remove or rename
/// fields, only adding fields is allowed or we risk breaking clients.
#[test]
#[cfg(not(feature = "nightly"))]
#[cfg_attr(feature = "nightly", ignore)]
fn test_runtime_config_view() {
use crate::runtime::config::RuntimeConfig;
use crate::runtime::config_store::RuntimeConfigStore;
use crate::views::RuntimeConfigView;
use near_primitives_core::version::PROTOCOL_VERSION;

// FIXME(#8202): This is snapshotting a config used for *tests*, rather than proper
// production configurations. That seems… subpar?
let config = RuntimeConfig::test();
let view = RuntimeConfigView::from(config);
let config_store = RuntimeConfigStore::new(None);
let config = config_store.get_config(PROTOCOL_VERSION);
let view = RuntimeConfigView::from(RuntimeConfig::clone(config));
insta::assert_json_snapshot!(&view);
}

/// `ExecutionMetadataView` with profile V1 displayed on the RPC should not change.
#[test]
#[cfg(not(feature = "nightly"))]
#[cfg_attr(feature = "nightly", ignore)]
fn test_exec_metadata_v1_view() {
let metadata = ExecutionMetadata::V1;
let view = ExecutionMetadataView::from(metadata);
Expand All @@ -2946,7 +2944,7 @@ mod tests {

/// `ExecutionMetadataView` with profile V2 displayed on the RPC should not change.
#[test]
#[cfg(not(feature = "nightly"))]
#[cfg_attr(feature = "nightly", ignore)]
fn test_exec_metadata_v2_view() {
let metadata = ExecutionMetadata::V2(ProfileDataV2::test());
let view = ExecutionMetadataView::from(metadata);
Expand All @@ -2955,7 +2953,7 @@ mod tests {

/// `ExecutionMetadataView` with profile V3 displayed on the RPC should not change.
#[test]
#[cfg(not(feature = "nightly"))]
#[cfg_attr(feature = "nightly", ignore)]
fn test_exec_metadata_v3_view() {
let metadata = ExecutionMetadata::V3(ProfileDataV3::test());
let view = ExecutionMetadataView::from(metadata);
Expand Down
3 changes: 2 additions & 1 deletion runtime/near-vm-runner/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,8 @@ impl crate::logic::Config {
ExtVMKind::Wasmer2 => VMKind::Wasmer2,
ExtVMKind::NearVm => VMKind::NearVm,
ExtVMKind::Wasmtime => VMKind::Wasmtime,
},
}
.normalize(),
disable_9393_fix: config.disable_9393_fix,
storage_get_mode: match config.storage_get_mode {
ExtStorageGetMode::Trie => StorageGetMode::Trie,
Expand Down
10 changes: 10 additions & 0 deletions runtime/near-vm-runner/src/vm_kind.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,3 +30,13 @@ pub enum VMKind {
/// NearVM.
NearVm,
}

impl VMKind {
pub fn normalize(self) -> Self {
if cfg!(not(target_arch = "x86_64")) {
Self::Wasmtime
} else {
self
}
}
}
3 changes: 0 additions & 3 deletions scripts/formatting
Original file line number Diff line number Diff line change
Expand Up @@ -31,9 +31,6 @@ def run(args: typing.Sequence[typing.Union[str, pathlib.Path]]) -> bool:
def run_tools(*, check: bool) -> str:
langs = []

if not run(('cargo', 'fmt') + (('--', '--check') if check else ())):
langs.append('Rust')

try:
import yapf as _
has_yapf = True
Expand Down
14 changes: 1 addition & 13 deletions scripts/run_clippy.sh
Original file line number Diff line number Diff line change
@@ -1,14 +1,2 @@
#!/usr/bin/env bash
# clippy adoption is in progress, see https://github.com/near/nearcore/issues/8145

LINTS=(
-A clippy::all
-D clippy::clone_on_copy
-D clippy::correctness
-D clippy::derivable_impls
-D clippy::redundant_clone
-D clippy::suspicious
-D clippy::len_zero
)

cargo clippy --all-targets -- "${LINTS[@]}"
exec cargo nextest run -p style-tests clippy
1 change: 1 addition & 0 deletions test-utils/style/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ fn clippy() {
cmd.args(&["clippy", "--all-targets", "--all-features", "--"]);
cmd.args(&[
"-Aclippy::all",
"-Dwarnings",
"-Dclippy::clone_on_copy",
"-Dclippy::correctness",
"-Dclippy::derivable_impls",
Expand Down

0 comments on commit 7a15a73

Please sign in to comment.