-
Notifications
You must be signed in to change notification settings - Fork 21
(MODULES-6582) Add ability to test against sql server 2017 #311
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
Conversation
444105b
to
995e4d6
Compare
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.
Just a couple nits. I'll hit the approve and you can change or ignore as you think best.
@@ -123,7 +123,7 @@ def sql_query_is_user_sysadmin(username) | |||
ensure_sqlserver_instance(features, inst_name, 'absent') | |||
end | |||
|
|||
it "create #{inst_name} instance with only one RS feature", :tier_high => true do | |||
it "create #{inst_name} instance with only one RS feature", :unless => version == '2017', :tier_high => true do |
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 we consider casting to an int
and doing a >=
comparison here, so we don't have to jigger this line the next time we want to add a version?
# TODO: Guard on SQL 2016 | ||
it "'ADV_SSMS'", :unless => sql_version == '2016', :tier_low => true do | ||
# TODO: Guard on SQL 2016 and 2017 | ||
it "'ADV_SSMS'", :unless => sql_version == '2016' || '2017', :tier_low => true do |
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.
Same here. This line already has two specific versions. A third would start to look cludgy wouldn't it?
995e4d6
to
4199d04
Compare
4199d04
to
b99f957
Compare
No description provided.