-
Notifications
You must be signed in to change notification settings - Fork 420
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
chore: Adjust integration tests after moving to separate package #2115
chore: Adjust integration tests after moving to separate package #2115
Conversation
Integration tests failure for d5a2ab72820865419a9a06fdb734b67548252bac |
Integration tests success for 8f2f53429bb52f42761e8f675da5c62f8e817204 |
@@ -15,13 +17,13 @@ func TestDatabaseRoleCreate(t *testing.T) { | |||
|
|||
t.Run("validation: nil options", func(t *testing.T) { | |||
var opts *createDatabaseRoleOptions = nil | |||
assertOptsInvalidJoinedErrors(t, opts, errNilOptions) | |||
assertOptsInvalidJoinedErrors(t, opts, ErrNilOptions) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should errors also be move into the internal package? Especially since jan is going to do some work with that. i guess it work make package imports weird. like errors.ErrNilOptions, also golang doesn't like it when you overwrite the standard package names
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would not go with moving them to internal because it may be a use case for SDK users to compare the error they got with the reference error to decide on the next steps of execution. That's why they are just exported, as we have discussed in #2111 (comment).
"github.com/stretchr/testify/require" | ||
) | ||
|
||
func Uuid(t *testing.T) string { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
my friend, this should be UUID
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we have another discussion coming. :D I strongly prefer having abbreviations written in only the first letter capitalized, but you are right; for now, our codebase has them capitalized, so I will rename them.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
Integration tests failure for 9ef8c9f475e4d0231c59a5674fee21dc50aca2ba |
Integration tests failure for 8ace52fd02b76cfde5e8ebed272f66bc5591cebf |
8ace52f
to
3d66a36
Compare
Integration tests failure for 3d66a3659ab0afb47e62a09389be22d278dadb9a |
Integration tests failure for 2c171d9820b4fee5a76a1a893a2bc2968013c6bf |
Integration tests failure for 2c171d9820b4fee5a76a1a893a2bc2968013c6bf |
Integration tests success for 2c171d9820b4fee5a76a1a893a2bc2968013c6bf |
This is the second PR to have SDK integration tests in a separate package. It is a continuation of #2111. It deals with the previous discussions and addresses the topics mentioned in the last PR.
Changes
integration_test_imports.go
file:random.go
filesrandom.go
from the integration tests package is entirely removedpkg/sdk/internal/random.go
sdk
packagerandomX
functions signatures were changed (t
parameter was removed)Next steps (for subsequent PRs) - summarized:
testClient
vs.itc.client
vs. top-levelclient
vs. something else?); the same with thetestContext
functiontestMain
methodTest Plan
References