Skip to content

don't run flaky CI in Windows #4935

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.

Sign up for GitHub

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

Merged
tersec merged 5 commits into from
May 15, 2023
Merged

don't run flaky CI in Windows #4935

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
3cfc7e0
don't run flaky CI in Windows
tersec May 10, 2023
9efc243
Merge branch 'unstable' into 5tC
etan-status May 12, 2023
1854506
reduce stack size in `test_light_client`
etan-status May 12, 2023
f036fc7
regen tests
etan-status May 13, 2023
154f9b9
re-enable maybe non-problematic test module on Windows
tersec May 14, 2023
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
6 changes: 4 additions & 2 deletions tests/all_tests.nim
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,9 @@ when not defined(i386):
import
./test_blockchain_dag,
./test_keystore,
./test_keystore_management,
./test_keymanager_api
Copy link
Copy Markdown
Contributor

@zah zah May 11, 2023

Choose a reason for hiding this comment

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

The flaky tests are test_keymanager_api and test_signing_node. test_keystore_management is mostly doing file I/O and test_keystore is mostly about the serialization and cryptography and these have been running smoothly.

Copy link
Copy Markdown
Contributor

@etan-status etan-status May 14, 2023

Choose a reason for hiding this comment

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

@zah, let's please prioritize fixing the tests then. Flaky CI is super time consuming for everyone (not just the maintainers of these flaky tests); the tests should be disabled in CI until they are capable of running reliably. When they run reliably again, just add them back to run on CI.

./test_keystore_management

when not defined(windows):
import ./test_keymanager_api

summarizeLongTests("AllTests")
34 changes: 19 additions & 15 deletions tests/test_light_client.nim
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -191,21 +191,25 @@ suite "Light client" & preset():
store.migrateToDataFork(lcDataFork)
withForkyStore(store):
when lcDataFork > LightClientDataFork.None:
bootstrap.migrateToDataFork(lcDataFork)
template forkyBootstrap: untyped = bootstrap.forky(lcDataFork)
let upgradedUpdate = update.migratingToDataFork(lcDataFork)
template forkyUpdate: untyped = upgradedUpdate.forky(lcDataFork)
let res = process_light_client_update(
forkyStore, forkyUpdate, currentSlot, cfg, genesis_validators_root)
check:
forkyUpdate.finalized_header.beacon.slot.sync_committee_period ==
period
res.isOk
if forkyUpdate.finalized_header.beacon.slot >
forkyBootstrap.header.beacon.slot:
forkyStore.finalized_header == forkyUpdate.finalized_header
else:
forkyStore.finalized_header == forkyBootstrap.header
# Reduce stack size by making this a `proc`
proc syncToPeriod() =
bootstrap.migrateToDataFork(lcDataFork)
template forkyBootstrap: untyped = bootstrap.forky(lcDataFork)
let upgradedUpdate = update.migratingToDataFork(lcDataFork)
template forkyUpdate: untyped = upgradedUpdate.forky(lcDataFork)
let res = process_light_client_update(
forkyStore, forkyUpdate, currentSlot, cfg,
genesis_validators_root)
check:
forkyUpdate.finalized_header.beacon.slot.sync_committee_period ==
period
res.isOk
if forkyUpdate.finalized_header.beacon.slot >
forkyBootstrap.header.beacon.slot:
forkyStore.finalized_header == forkyUpdate.finalized_header
else:
forkyStore.finalized_header == forkyBootstrap.header
syncToPeriod()
inc numIterations
if numIterations > 20: doAssert false # Avoid endless loop on test failure

Expand Down