pendulum-chain / spacewalk

Apache License 2.0
34 stars 7 forks source link

Improve robustness of Spacewalk integration tests #495

Closed ebma closed 5 months ago

ebma commented 6 months ago

Closes #494.

The multiple mainnet integration issues (and its fixes) are as follows:

b-yap commented 6 months ago

one test is failing with:

thread 'test_redeem_succeeds_on_mainnet' panicked at 'should return ok: HorizonSubmissionError { title: "Transaction Failed", status: 400, reason: "tx_failed", result_code_op: ["op_underfunded"], envelope_xdr: Some("AAAAAgAAAACne8I5bB4KhDOubC5DCq7O/p4yvnXbjNkOE8fT3v0MHwAAAGUC7t8CAAABLwAAAAAAAAABAAAAHDNCMWdEd2U1YTZUS2dFcHVmdmJZcmRCcVZWNHYAAAABAAAAAQAAAACne8I5bB4KhDOubC5DCq7O/p4yvnXbjNkOE8fT3v0MHwAAAAEAAAAArwuPZBi/hC2gPFK8R7thoU5Xsbh+k5n1ls8DrCkdhJMAAAABVVNEQwAAAAA7mRE4Dv6Yi6CokA6xz+RPNm99vpRr7QdyQPf2JN8VxQAAAAAAAYagAAAAAAAAAAHe/QwfAAAAQElB9VXUO8NynqXVQVrdXuAECle2Iebo48ciDX+d5T0sMq3kz7YVDQqcw3uvPftSmCtRXLDoP2sAleJBIcNopQg=") }', clients/vault/tests/helper/helper.rs:196:6

the address GCTXXQRZNQPAVBBTVZWC4QYKV3HP5HRSXZ25XDGZBYJ4PU667UGB642R doesn't have any balance anymore.

ebma commented 5 months ago

Now the CI job failed with

thread 'test_redeem_succeeds_on_mainnet' panicked at 'should return ok: HorizonSubmissionError { title: "Transaction Failed", status: 400, reason: "tx_insufficient_fee", result_code_op: [], 

which is weird because recently already changed the default fee percentile to be 95 instead of 90. We could set it to 100 but then we potentially pay a very large fee :(

b-yap commented 5 months ago

@ebma
I think it might do us good if we have more than 2 secret keys for testing; Like 5 keys (and I meant on testnet network). Separate keys per lib/module/package. There's the stellar-relay-lib, wallet, vault.

ebma commented 5 months ago

It might help yes. But does it make sense? Shouldn't in a perfect world our tests be able to just work with one set of accounts instead of us having to rely on switching them randomly?

b-yap commented 5 months ago

you are right. :/

b-yap commented 5 months ago

@ebma For macos it's definitely a coincidence. I'll be keeping my eye on the mainnet tests.

ebma commented 5 months ago

I know I already brought this up a very long time ago, but how about we just remove the macOS job from the workflow completely? I don't see much of a benefit here, besides our workflows taking twice the time and being more likely to fail in some sense. Of course, removing something just because it doesn't work is not the ideal solution but generally, even assuming that the macOS job worked reliably all the time, I'd argue that vault operators would deploy these vaults in a Linux environment and not on a macOS machine so it should be fine to only test the workflow on Ubuntu.

WDYT @b-yap? Do you want to keep it or should we remove it?

b-yap commented 5 months ago

@ebma I'd vote on removing it.

ebma commented 5 months ago

Let's remove it and merge this without waiting for the checks