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

CLOUDP-294932: Fix contract test skips #2049

Merged
merged 3 commits into from
Jan 16, 2025
Merged

Conversation

josvazg
Copy link
Collaborator

@josvazg josvazg commented Jan 15, 2025

Fix bug skipping all contract tests by default and add unit tests to verify fix.

All Submissions:

  • Have you signed our CLA?

@josvazg josvazg changed the title Fix contract test skips CLOUDP-294932: Fix contract test skips Jan 15, 2025
@josvazg
Copy link
Collaborator Author

josvazg commented Jan 15, 2025

CLOUDP-294932

@josvazg josvazg force-pushed the CLOUDP-294932/fix-contract-skips branch from 7ed2616 to 8f6fa5b Compare January 15, 2025 14:55
@@ -68,6 +61,16 @@ func RunGoContractTest(ctx context.Context, t *testing.T, name string, contractT
})
}

func skipCheck(name, focus string, enabled bool) string {
Copy link
Collaborator

@helderjs helderjs Jan 15, 2025

Choose a reason for hiding this comment

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

Wouldn't (bool, string) or simply error be a more idiomatic type return?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

yes, error seems better

Signed-off-by: jose.vazquez <[email protected]>
@josvazg josvazg force-pushed the CLOUDP-294932/fix-contract-skips branch from 8f6fa5b to cac9bcd Compare January 15, 2025 15:22
Copy link
Collaborator

@s-urbaniak s-urbaniak left a comment

Choose a reason for hiding this comment

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

one nit else lgtm

josvazg and others added 2 commits January 16, 2025 09:29
Use simpler non parameterised error constructor

Co-authored-by: Sergiusz Urbaniak <[email protected]>
Signed-off-by: jose.vazquez <[email protected]>
@josvazg josvazg merged commit 8b2869c into main Jan 16, 2025
9 checks passed
@josvazg josvazg deleted the CLOUDP-294932/fix-contract-skips branch January 16, 2025 09:37
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.

3 participants