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

Assert SDK version is consistent in the CLI generation process #1814

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

shreyas-goenka
Copy link
Contributor

@shreyas-goenka shreyas-goenka commented Oct 7, 2024

Changes

Followup from #1809 (comment). User will see the following error if the SDK version does not match during CLI code generation.

--- FAIL: TestConsistentDatabricksSdkVersion (1.34s)
    info_test.go:53: 
                Error Trace:    /Users/shreyas.goenka/cli/internal/build/info_test.go:53
                Error:          Not equal: 
                                expected: "0c86ea6dbd9a730c24ff0d4e509603e476955ac5"
                                actual  : "6f6b1371e640f2dfeba72d365ac566368656f6b6"
                            
                                Diff:
                                --- Expected
                                +++ Actual
                                @@ -1 +1 @@
                                -0c86ea6dbd9a730c24ff0d4e509603e476955ac5
                                +6f6b1371e640f2dfeba72d365ac566368656f6b6
                Test:           TestConsistentDatabricksSdkVersion
                Messages:       please update the SDK version before generating the CLI

Tests

Manually asserted that:

  1. Generating the CLI without the SDK bump causes an error.
  2. Generating the CLI after the SDK bump works as expected.
  3. The test works even when the SDK is pinned to a specific commit like v0.47.1-0.20241002195128-6cecc224cbf7

internal/build/info_test.go Show resolved Hide resolved
internal/build/info_test.go Outdated Show resolved Hide resolved
internal/build/info_test.go Outdated Show resolved Hide resolved
Copy link
Contributor

@pietern pietern left a comment

Choose a reason for hiding this comment

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

Can you figure out how we can not run this test by default?

Maybe with build tags or something.

internal/build/info_test.go Outdated Show resolved Hide resolved
@shreyas-goenka
Copy link
Contributor Author

@pietern I think we can skip a filter on this test for now. It's pretty fast, it ran 100 times in 3.7 seconds on my local machine.

It's difficult to determine whether we are regenerating the CLI in the context of the CLI code itself. The build tags are only set in the release workflow. We'd probably need to create some sort of flag in the push.yml workflow to note when a PR is regenerating the CLI.

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.

2 participants