-
Notifications
You must be signed in to change notification settings - Fork 4
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
test: Add connection v1 tests #104
Conversation
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 am approving this PR because but I do not have enough expertise in this code. Please take a look on my comments too.
}), | ||
// Get account id | ||
rest.get( | ||
`https://${apiEndpoint}/iam/v2/accounts:getIdByName`, |
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.
shouldn't URL contain query parameter accountName=blblblbla
?
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.
It doesn't need to since otherwise it accepts all parameters. However, we sure can check that it was passed correctly. I've added an expect
statement.
} | ||
), | ||
// Get default account | ||
rest.get(`https://${apiEndpoint}/iam/v2/account`, (req, res, ctx) => { |
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.
at least in JDBC and .NET we use core/v1/account
and core/v1/accounts
for this is purpose. Is there a chance that I have to change the JDBC and .NET or these tests are wrong?
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.
Are you sure these drivers use core/v1
endpoints for the same operations? This is for getting default account (/iam/v2/account
) or resolving account name to an id (iam/v2/accounts:getIdByName
)
Firebolt 2.0 uses web/v3/account
for these operations btw.
Quality Gate passedIssues Measures |
Some basic tests to improve coverage.