Skip to content

(FM-3655) SQL Server CI acceptance issues #151

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

Merged

Conversation

phongdly
Copy link
Contributor

Before this PR, database name is a constant value and with the pending issue of unable to delete database (ticket MODULES-2554), only one database has been used in several tests. This cause the test CI failed.

This PR removes all constants and replaced them by variables, commenting out dead codes, and temporarily skip delete/absent database in each test script (because of MODULES-2554 as mentioned above)

@cyberious
Copy link
Contributor

LGTM 👍

cyberious added a commit that referenced this pull request Sep 14, 2015
…e_issues

(FM-3655) SQL Server CI acceptance issues
@cyberious cyberious merged commit ec4dc83 into puppetlabs:master Sep 14, 2015
@phongdly phongdly deleted the FM-3655/SQL_Server_CI_acceptance_issues branch September 14, 2015 16:47
ensure_sqlserver_database(host, 'absent')
# remove the newly created database
# Temporarily skip delete database because of MODULES-2554
#ensure_sqlserver_database(host, 'absent')
Copy link
Contributor

Choose a reason for hiding this comment

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

This is probably OK now - but cleanup might be necessary if the suite size grows / too many temporary databases are created.

Copy link
Contributor

Choose a reason for hiding this comment

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

It would have been nice to emit a warning here as well that the suite has orphaned a database @phongdly

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