Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

Add async test helper to timeout and provide a task_executor automatically #6651

Merged
30 commits merged into from
Aug 12, 2020
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
30 commits
Select commit Hold shift + click to select a range
476c881
Initial commit
cecton Jul 14, 2020
8117934
Add async test helper to timeout and provide a task_executor automati…
cecton Jul 14, 2020
ce5ea42
Merge commit 056879f376c46154847927928511c6fd127bef28 (no conflict)
cecton Jul 22, 2020
34c0f23
Merge commit aa36bf284178daaea56399fedf5fcae8b9e282bc (conflicts)
cecton Jul 22, 2020
544c1d2
Merge commit 64d4a4da2a62b59bf4f0212174149c31292b62e7 (no conflict)
cecton Jul 22, 2020
2796798
simplify error message to avoid difference between CI and locally
cecton Jul 22, 2020
a0535f7
forgot env var
cecton Jul 22, 2020
b86cc8e
Use runtime env var instead of build env var
cecton Jul 22, 2020
6b2f66f
Rename variable to SUBSTRATE_TEST_TIMEOUT
cecton Jul 22, 2020
2a47de6
CLEANUP
cecton Jul 22, 2020
24a165b
Apply suggestions from code review
cecton Jul 22, 2020
0507b2f
Merge commit edb48cfdd90e2658017e696a33ae566c7e0940a4 (no conflict)
cecton Jul 23, 2020
f173c1c
Re-export from test-utils
cecton Jul 24, 2020
8e45871
Default value to 120
cecton Jul 24, 2020
b7e1b1d
fix wrong crate in ci
cecton Jul 24, 2020
a200f34
Revert "Default value to 120"
cecton Jul 24, 2020
c157c22
Fix version
cecton Aug 6, 2020
d074de0
WIP
cecton Aug 6, 2020
f0c0e7f
WIP
cecton Aug 6, 2020
3a5d4f0
WIP
cecton Aug 6, 2020
40d1804
remove feature flag
cecton Aug 6, 2020
a8c62ff
fix missing dependency
cecton Aug 7, 2020
82ceab0
CLEANUP
cecton Aug 7, 2020
badbec3
fix test
cecton Aug 7, 2020
a9d03e1
Removed autotests=false
cecton Aug 12, 2020
c7c3e52
Some doc...
cecton Aug 12, 2020
78037b8
Apply suggestions from code review
cecton Aug 12, 2020
0587d76
WIP
cecton Aug 12, 2020
e4b9ff1
WIP
cecton Aug 12, 2020
948c64c
Update test-utils/src/lib.rs
bkchr Aug 12, 2020
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion .gitlab-ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -230,6 +230,7 @@ test-linux-stable: &test-linux
script:
# this job runs all tests in former runtime-benchmarks, frame-staking and wasmtime tests
- time cargo test --workspace --locked --release --verbose --features runtime-benchmarks --manifest-path bin/node/cli/Cargo.toml
- WASM_BUILD_NO_COLOR=1 INTEGRATION_TEST_ALLOWED_TIME=1 time cargo test -p substrate-test-utils-derive --release --verbose --locked -- --ignored timeout
- sccache -s

unleash-check:
Expand Down Expand Up @@ -621,7 +622,7 @@ deploy-kubernetes-alerting-rules:
RULES: .maintain/monitoring/alerting-rules/alerting-rules.yaml
script:
- echo "deploying prometheus alerting rules"
- kubectl -n ${NAMESPACE} patch prometheusrule ${PROMETHEUSRULE}
- kubectl -n ${NAMESPACE} patch prometheusrule ${PROMETHEUSRULE}
--type=merge --patch "$(sed 's/^/ /;1s/^/spec:\n/' ${RULES})"
only:
refs:
Expand Down
19 changes: 19 additions & 0 deletions Cargo.lock

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

1 change: 1 addition & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -171,6 +171,7 @@ members = [
"primitives/utils",
"primitives/wasm-interface",
"test-utils/client",
"test-utils/derive",
"test-utils/runtime",
"test-utils/runtime/client",
"test-utils/runtime/transaction-pool",
Expand Down
30 changes: 30 additions & 0 deletions test-utils/derive/Cargo.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
[package]
name = "substrate-test-utils-derive"
mihir-ghl marked this conversation as resolved.
Show resolved Hide resolved
version = "0.8.0-rc4"
cecton marked this conversation as resolved.
Show resolved Hide resolved
authors = ["Parity Technologies <admin@parity.io>"]
edition = "2018"
license = "Apache-2.0"
homepage = "https://substrate.dev"
repository = "https://github.com/paritytech/substrate/"
autotests = false

[dependencies]
quote = "1"
gnunicorn marked this conversation as resolved.
Show resolved Hide resolved
syn = { version = "1", default-features = false }
gnunicorn marked this conversation as resolved.
Show resolved Hide resolved

[dev-dependencies]
tokio = { version = "0.2.13", features = ["macros"] }
futures = { version = "0.3.1", features = ["compat"] }
sc-service = { path = "../../client/service" }
trybuild = { version = "1.0", features = ["diff"] }

[lib]
proc-macro = true

[[test]]
name = "basic"
path = "tests/basic.rs"

[[test]]
name = "errors"
path = "tests/errors.rs"
101 changes: 101 additions & 0 deletions test-utils/derive/src/lib.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,101 @@
// This file is part of Substrate.

// Copyright (C) 2020 Parity Technologies (UK) Ltd.
// SPDX-License-Identifier: GPL-3.0-or-later WITH Classpath-exception-2.0

// This program is free software: you can redistribute it and/or modify
// it under the terms of the GNU General Public License as published by
// the Free Software Foundation, either version 3 of the License, or
// (at your option) any later version.

// This program is distributed in the hope that it will be useful,
// but WITHOUT ANY WARRANTY; without even the implied warranty of
// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
// GNU General Public License for more details.

// You should have received a copy of the GNU General Public License
// along with this program. If not, see <https://www.gnu.org/licenses/>.

use proc_macro::TokenStream;
use quote::quote;

#[proc_macro_attribute]
pub fn test(
args: proc_macro::TokenStream,
item: proc_macro::TokenStream,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You import it above.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed

) -> proc_macro::TokenStream {
impl_test(args, item)
}

fn impl_test(
args: proc_macro::TokenStream,
item: proc_macro::TokenStream,
) -> proc_macro::TokenStream {
let input = syn::parse_macro_input!(item as syn::ItemFn);
let args = syn::parse_macro_input!(args as syn::AttributeArgs);

parse_knobs(input, args).unwrap_or_else(|e| e.to_compile_error().into())
}

fn parse_knobs(
mut input: syn::ItemFn,
args: syn::AttributeArgs,
) -> Result<TokenStream, syn::Error> {
let sig = &mut input.sig;
let body = &input.block;
let attrs = &input.attrs;
let vis = input.vis;

if sig.inputs.len() != 1 {
let msg = "the test function accepts only one argument of type sc_service::TaskExecutor";
return Ok(syn::Error::new_spanned(&sig, msg).to_compile_error().into());
cecton marked this conversation as resolved.
Show resolved Hide resolved
}
let (task_executor_name, task_executor_type) = match sig.inputs.pop().map(|x| x.into_value()) {
Some(syn::FnArg::Typed(x)) => (x.pat, x.ty),
_ => {
let msg =
"the test function accepts only one argument of type sc_service::TaskExecutor";
return Ok(syn::Error::new_spanned(&sig, msg).to_compile_error().into());
cecton marked this conversation as resolved.
Show resolved Hide resolved
}
};

let header = {
quote! {
#[tokio::test(#(#args)*)]
}
};

let result = quote! {
#header
#(#attrs)*
#vis #sig {
use futures::future::FutureExt;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a no go pattern in macros.

Macros should import required crates over the crate they are exported.

Copy link
Contributor Author

@cecton cecton Jul 22, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you're right for macro_rules but this is a proc_macro crate, I can't re-export things like that:

error: `proc-macro` crate types currently cannot export any items other than functions tagged with `#[proc_macro]`, `#[proc_macro_derive]`, or `#[proc_macro_attribute]`
  --> test-utils/derive/src/lib.rs:21:1
   |
21 | pub use tokio;
   | ^^^^^^^^^^^^^^

error: `proc-macro` crate types currently cannot export any items other than functions tagged with `#[proc_macro]`, `#[proc_macro_derive]`, or `#[proc_macro_attribute]`
  --> test-utils/derive/src/lib.rs:22:1
   |
22 | pub use futures;
   | ^^^^^^^^^^^^^^^^

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will investigate how this is usually done for proc-macros

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah yeah you said it but I misread. Ok I will re-export from test-utils

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done


let #task_executor_name: #task_executor_type = (|fut, _| {
tokio::spawn(fut).map(drop)
})
.into();
let timeout_task = tokio::time::delay_for(
std::time::Duration::from_secs(
option_env!("INTEGRATION_TEST_ALLOWED_TIME")
gnunicorn marked this conversation as resolved.
Show resolved Hide resolved
gnunicorn marked this conversation as resolved.
Show resolved Hide resolved
.and_then(|x| x.parse().ok())
.unwrap_or(600))
).fuse();
let actual_test_task = async move {
#body
}
.fuse();

futures::pin_mut!(timeout_task, actual_test_task);

futures::select! {
_ = timeout_task => {
panic!("the test took too long");
cecton marked this conversation as resolved.
Show resolved Hide resolved
},
_ = actual_test_task => {},
}
}
};

Ok(result.into())
}
58 changes: 58 additions & 0 deletions test-utils/derive/tests/basic.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
// This file is part of Substrate.

// Copyright (C) 2020 Parity Technologies (UK) Ltd.
// SPDX-License-Identifier: GPL-3.0-or-later WITH Classpath-exception-2.0

// This program is free software: you can redistribute it and/or modify
// it under the terms of the GNU General Public License as published by
// the Free Software Foundation, either version 3 of the License, or
// (at your option) any later version.

// This program is distributed in the hope that it will be useful,
// but WITHOUT ANY WARRANTY; without even the implied warranty of
// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
// GNU General Public License for more details.

// You should have received a copy of the GNU General Public License
// along with this program. If not, see <https://www.gnu.org/licenses/>.

use sc_service::{TaskExecutor, TaskType};

#[substrate_test_utils_derive::test]
async fn basic_test(_: TaskExecutor) {
assert!(true);
}

#[substrate_test_utils_derive::test]
#[should_panic(expected = "boo!")]
async fn panicking_test(_: TaskExecutor) {
panic!("boo!");
}

#[substrate_test_utils_derive::test(max_threads = 2)]
async fn basic_test_with_args(_: TaskExecutor) {
assert!(true);
}

#[substrate_test_utils_derive::test]
async fn rename_argument(ex: TaskExecutor) {
let ex2 = ex.clone();
ex2.spawn(Box::pin(async { () }), TaskType::Blocking);
assert!(true);
}

#[substrate_test_utils_derive::test]
#[should_panic(expected = "test took too long")]
// NOTE: enable this test only after setting INTEGRATION_TEST_ALLOWED_TIME to a smaller value
//
// INTEGRATION_TEST_ALLOWED_TIME=1 cargo test -- --ignored timeout
#[ignore]
async fn timeout(_: TaskExecutor) {
tokio::time::delay_for(std::time::Duration::from_secs(
option_env!("INTEGRATION_TEST_ALLOWED_TIME")
.expect("env var INTEGRATION_TEST_ALLOWED_TIME has been provided by the user")
.parse::<u64>()
.unwrap() + 1,
))
.await;
}
24 changes: 24 additions & 0 deletions test-utils/derive/tests/errors.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
// This file is part of Substrate.

// Copyright (C) 2020 Parity Technologies (UK) Ltd.
// SPDX-License-Identifier: GPL-3.0-or-later WITH Classpath-exception-2.0

// This program is free software: you can redistribute it and/or modify
// it under the terms of the GNU General Public License as published by
// the Free Software Foundation, either version 3 of the License, or
// (at your option) any later version.

// This program is distributed in the hope that it will be useful,
// but WITHOUT ANY WARRANTY; without even the implied warranty of
// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
// GNU General Public License for more details.

// You should have received a copy of the GNU General Public License
// along with this program. If not, see <https://www.gnu.org/licenses/>.

#[test]
fn substrate_test_utils_derive_trybuild() {
let t = trybuild::TestCases::new();
t.compile_fail("tests/missing-func-parameter.rs");
t.compile_fail("tests/too-many-func-parameters.rs");
}
24 changes: 24 additions & 0 deletions test-utils/derive/tests/missing-func-parameter.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
// This file is part of Substrate.

// Copyright (C) 2020 Parity Technologies (UK) Ltd.
// SPDX-License-Identifier: GPL-3.0-or-later WITH Classpath-exception-2.0

// This program is free software: you can redistribute it and/or modify
// it under the terms of the GNU General Public License as published by
// the Free Software Foundation, either version 3 of the License, or
// (at your option) any later version.

// This program is distributed in the hope that it will be useful,
// but WITHOUT ANY WARRANTY; without even the implied warranty of
// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
// GNU General Public License for more details.

// You should have received a copy of the GNU General Public License
// along with this program. If not, see <https://www.gnu.org/licenses/>.

#[substrate_test_utils_derive::test]
async fn missing_func_parameter() {
assert!(true);
}

fn main() {}
5 changes: 5 additions & 0 deletions test-utils/derive/tests/missing-func-parameter.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
error: the test function accepts only one argument of type sc_service::TaskExecutor
--> $DIR/missing-func-parameter.rs:20:1
|
20 | async fn missing_func_parameter() {
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
27 changes: 27 additions & 0 deletions test-utils/derive/tests/too-many-func-parameters.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
// This file is part of Substrate.

// Copyright (C) 2020 Parity Technologies (UK) Ltd.
// SPDX-License-Identifier: GPL-3.0-or-later WITH Classpath-exception-2.0

// This program is free software: you can redistribute it and/or modify
// it under the terms of the GNU General Public License as published by
// the Free Software Foundation, either version 3 of the License, or
// (at your option) any later version.

// This program is distributed in the hope that it will be useful,
// but WITHOUT ANY WARRANTY; without even the implied warranty of
// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
// GNU General Public License for more details.

// You should have received a copy of the GNU General Public License
// along with this program. If not, see <https://www.gnu.org/licenses/>.

#[allow(unused_imports)]
use sc_service::TaskExecutor;

#[substrate_test_utils_derive::test]
async fn too_many_func_parameters(task_executor_1: TaskExecutor, task_executor_2: TaskExecutor) {
assert!(true);
}

fn main() {}
5 changes: 5 additions & 0 deletions test-utils/derive/tests/too-many-func-parameters.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
error: the test function accepts only one argument of type sc_service::TaskExecutor
--> $DIR/too-many-func-parameters.rs:23:1
|
23 | async fn too_many_func_parameters(task_executor_1: TaskExecutor, task_executor_2: TaskExecutor) {
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^