Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Transaction status events tested for each of 7 peers #1631

Merged
merged 1 commit into from Nov 24, 2021

Conversation

e-ivkov
Copy link
Contributor

@e-ivkov e-ivkov commented Nov 23, 2021

Signed-off-by: Egor Ivkov e.o.ivkov@gmail.com

Description of the Change

Transaction status events are tested:

  • By connecting for events to each peer
  • By submitting transactions in turn to each peer

Issue

#1492

Benefits

Thoroughly testing potential bug cases described in the issue.

@e-ivkov e-ivkov self-assigned this Nov 23, 2021
@github-actions github-actions bot added the iroha2 The re-implementation of a BFT hyperledger in RUST label Nov 23, 2021
@codecov
Copy link

codecov bot commented Nov 23, 2021

Codecov Report

Merging #1631 (b49254b) into iroha2-dev (3285d53) will increase coverage by 1.11%.
The diff coverage is 98.78%.

❗ Current head b49254b differs from pull request most recent head 8f6f77a. Consider uploading reports for the commit 8f6f77a to get more accurate results
Impacted file tree graph

@@              Coverage Diff               @@
##           iroha2-dev    #1631      +/-   ##
==============================================
+ Coverage       74.75%   75.86%   +1.11%     
==============================================
  Files             120      119       -1     
  Lines           19803    19671     -132     
==============================================
+ Hits            14803    14923     +120     
+ Misses           5000     4748     -252     
Impacted Files Coverage Δ
client/tests/tests/events.rs 98.79% <98.70%> (-1.21%) ⬇️
core/src/sumeragi/mod.rs 88.66% <100.00%> (+6.08%) ⬆️
core/test_network/src/lib.rs 74.82% <100.00%> (-1.28%) ⬇️
p2p/src/network.rs 78.94% <100.00%> (ø)
actor/src/lib.rs 81.87% <0.00%> (-0.33%) ⬇️
crypto/src/lib.rs 89.54% <0.00%> (-0.11%) ⬇️
core/src/lib.rs 62.11% <0.00%> (ø)
client/src/lib.rs 100.00% <0.00%> (ø)
... and 20 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3285d53...8f6f77a. Read the comment docs.

core/test_network/src/lib.rs Outdated Show resolved Hide resolved
core/test_network/src/lib.rs Outdated Show resolved Hide resolved
docs/source/references/api_spec.md Outdated Show resolved Hide resolved
docs/source/references/api_spec.md Outdated Show resolved Hide resolved
docs/source/references/api_spec.md Outdated Show resolved Hide resolved
docs/source/references/api_spec.md Outdated Show resolved Hide resolved
docs/source/references/api_spec.md Outdated Show resolved Hide resolved
docs/source/references/api_spec.md Outdated Show resolved Hide resolved
client/tests/tests/events.rs Outdated Show resolved Hide resolved
client/tests/tests/events.rs Show resolved Hide resolved

let committed_event_received = Arc::new(RwLock::new([false; PEER_COUNT]));
let validating_event_received = Arc::new(RwLock::new([false; PEER_COUNT]));
let rejected_event_received = Arc::new(RwLock::new([false; PEER_COUNT]));
Copy link
Contributor

Choose a reason for hiding this comment

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

funny thing, these array locations are not accessed concurrently, each thread accesses different index. But that would be a pain to express correctly

Copy link
Contributor

@mversic mversic Nov 23, 2021

Choose a reason for hiding this comment

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

yes, you can have:

const INIT: AtomicBool = AtomicBool::new(false);
let rejected_event_received = Arc::new([INIT; PEER_COUNT]); 

if you think that's worth the trouble for this test?

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 think it doesn't matter here that much as it's just a test.

mversic
mversic previously approved these changes Nov 24, 2021
appetrosyan
appetrosyan previously approved these changes Nov 24, 2021
Signed-off-by: Egor Ivkov <e.o.ivkov@gmail.com>
@mversic mversic self-requested a review November 24, 2021 13:37
@e-ivkov e-ivkov merged commit 1b6bcb0 into hyperledger:iroha2-dev Nov 24, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
iroha2 The re-implementation of a BFT hyperledger in RUST
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants