Skip to content

chore: enable ignored tests#1878

Merged
gilcu3 merged 8 commits intomainfrom
1150-fix-and-re-enable-broken-contract-tee-tests
Jan 28, 2026
Merged

chore: enable ignored tests#1878
gilcu3 merged 8 commits intomainfrom
1150-fix-and-re-enable-broken-contract-tee-tests

Conversation

@gilcu3
Copy link
Contributor

@gilcu3 gilcu3 commented Jan 27, 2026

Closes #1150

@gilcu3 gilcu3 linked an issue Jan 27, 2026 that may be closed by this pull request
@gilcu3 gilcu3 force-pushed the 1150-fix-and-re-enable-broken-contract-tee-tests branch from 4a72682 to 7283efb Compare January 27, 2026 11:50
attestation: &Attestation,
tls_key: &Ed25519PublicKey,
) -> anyhow::Result<bool> {
) -> anyhow::Result<ExecutionFinalResult> {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this change is not strictly needed anymore, but seems like a good practice so that the function can also be used to match specific errors in case of failure

Cargo.toml Outdated
"serde",
], default-features = false }
dcap-qvl = { version = "0.3.10", default-features = false, features = ["contract", "borsh", "std"] }
dcap-qvl = { version = "0.3.10", default-features = false, features = ["contract", "borsh", "std", "ring"] }
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this is also not strictly needed to solve the issue, as we now use unit tests. But still without this attestations would fail, see Phala-Network/dcap-qvl#99

Copy link
Member

Choose a reason for hiding this comment

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

The ring feature is supported since v0.3.11. You may need to update the dcap-qvl version.

Copy link
Contributor Author

@gilcu3 gilcu3 Jan 27, 2026

Choose a reason for hiding this comment

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

oh, that was the point so this was probably a typo on my side
EDIT: indeed Cargo.lock had been updated, else it shouldn't have worked 😄

@gilcu3 gilcu3 marked this pull request as ready for review January 27, 2026 16:08
barakeinav1
barakeinav1 previously approved these changes Jan 28, 2026
Copy link
Contributor

@barakeinav1 barakeinav1 left a comment

Choose a reason for hiding this comment

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

looks good. thanks.
one tiny comment

DSharifi
DSharifi previously approved these changes Jan 28, 2026
Copy link
Contributor

@DSharifi DSharifi left a comment

Choose a reason for hiding this comment

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

Thanks for enabling these tests!

I have some small nits, but otherwise looks very good 💯

@gilcu3 gilcu3 enabled auto-merge January 28, 2026 09:35
@gilcu3 gilcu3 added this pull request to the merge queue Jan 28, 2026
Merged via the queue into main with commit 832c321 Jan 28, 2026
7 checks passed
@gilcu3 gilcu3 deleted the 1150-fix-and-re-enable-broken-contract-tee-tests branch January 28, 2026 10:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Fix and re-enable broken contract tee tests

4 participants