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.

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

Jump to bottom

ledger: fix tests #1045

Merged
merged 2 commits into from
Jan 23, 2023
Merged

ledger: fix tests #1045

merged 2 commits into from
Jan 23, 2023

Conversation

Fraccaman
Copy link
Member

@Fraccaman Fraccaman commented Jan 17, 2023 •
edited
Loading

  • Fixes an issue with the e2e scheduler script not exiting with the right status code when a test was failing
  • Fixes unit and e2e test not passing

@Fraccaman Fraccaman changed the title ci: forgot exit code i ci: fix exit code in ci script Jan 17, 2023
grarco
grarco previously approved these changes Jan 17, 2023
View reviewed changes
@tzemanovic tzemanovic mentioned this pull request Jan 18, 2023
Merged
@Fraccaman Fraccaman force-pushed the fraccaman/fix-e2e-scheduler-ci branch from 21ab0d6 to 4ceed5d Compare January 19, 2023 16:11
@Fraccaman Fraccaman changed the title ci: fix exit code in ci script ledger: fix tests Jan 19, 2023
@Fraccaman Fraccaman marked this pull request as ready for review January 19, 2023 16:13
grarco
grarco previously approved these changes Jan 19, 2023
View reviewed changes
tzemanovic
tzemanovic reviewed Jan 20, 2023
View reviewed changes
tests/src/e2e/ledger_tests.rs Outdated
@@ -126,7 +126,7 @@ fn test_node_connectivity_and_consensus() -> Result<()> {
// 4. Check that all the nodes processed the tx with the same result
let mut validator_0 = bg_validator_0.foreground();
let mut validator_1 = bg_validator_1.foreground();
let expected_result = "all VPs accepted transaction";
let expected_result = "successful txs: 1";
Copy link
Member

Choose a reason for hiding this comment

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

I think there's an issue with this (and we actually already have it in some tests) that is, 1 tx will appear twice in "successful txs", once for the wrapper and then inner tx. Some of the existing tests that are waiting for "Transaction is valid" have the same issue

Copy link
Member

Choose a reason for hiding this comment

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

I think ideally the wrapper tx result should use a different type and format differently in the client (related to #20) and maybe we should distinguish these in the ledger stats log too

Copy link
Member Author

Choose a reason for hiding this comment

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

yeah, I agree, ill try to open a new PR to improve e2e

tzemanovic
tzemanovic reviewed Jan 20, 2023
View reviewed changes
wasm/wasm_source/src/vp_implicit.rs Outdated
@@ -710,7 +710,8 @@ mod tests {
std::fs::read(VP_ALWAYS_TRUE_WASM).expect("cannot load wasm");

// hardcoded hash of VP_ALWAYS_TRUE_WASM
tx_env.init_parameters(None, None, Some(vec!["E3B0C44298FC1C149AFBF4C8996FB92427AE41E4649B934CA495991B7852B855".to_string()]));
// this must be lower case, as its written into genesis as lower case
tx_env.init_parameters(None, None, Some(vec!["e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855".to_string()]));
Copy link
Member

Choose a reason for hiding this comment

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

I don't think these changes are necessary as the init_paramaters calls to the params update functions that apply lowercasing now

@juped juped force-pushed the fraccaman/fix-e2e-scheduler-ci branch from 6b10565 to 94431c8 Compare January 20, 2023 16:31
@juped
Copy link
Member

juped commented Jan 20, 2023

(above test failures expected here)

juped added a commit that referenced this pull request Jan 23, 2023
@juped
Sync with 0.13.2
3b57e84 
* tag 'v0.13.2':
  Namada 0.13.2
  changelog: add #1057
  ledger: append version to moniker
  changelog: add #1062
  changelog: add #1045
  fix: unit/e2e test
  changelog: add #1019
  Fixes wrapper fee check
  fix: ci e2e scheduler script
@juped juped merged commit 84cea19 into main Jan 23, 2023
@juped juped deleted the fraccaman/fix-e2e-scheduler-ci branch January 23, 2023 17:54
bengtlofgren pushed a commit that referenced this pull request May 11, 2023
@juped @bengtlofgren
changelog: add #1045
a0970f7
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tzemanovic tzemanovic tzemanovic left review comments

@grarco grarco grarco left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@Fraccaman @juped @tzemanovic @grarco