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

ext/mysqli: Skip test that needs superuser #17531

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

NattyNarwhal
Copy link
Member

This test needs CREATE DATABASE and CREATE SERVER, and will fail in the cleanup step if it doesn't have it. If the test is running as a user that can't do these, then we should skip instead of borking.

Alternative to GH-17466.

This test needs CREATE DATABASE and CREATE SERVER, and will fail in
the cleanup step if it doesn't have it. If the test is running as a
user that can't do these, then we should skip instead of borking.

Alternative to phpGH-17466.
@kamil-tekiela
Copy link
Member

Can you explain why is useful to run the tests with a user who doesn't have the full permissions? What does it accomplish?

@NattyNarwhal
Copy link
Member Author

Setting up a CI environment that isn't (yet) containerized; see GH-17258. Because of that, I'm a little hesitant to grant the CI DB user more than all privileges on a specific DB. That said, maybe GRANT CREATE, DROP ON *.* might not be too bad.

@kamil-tekiela
Copy link
Member

Setting up a CI environment that isn't (yet) containerized; see GH-17258. Because of that, I'm a little hesitant to grant the CI DB user more than all privileges on a specific DB. That said, maybe GRANT CREATE, DROP ON *.* might not be too bad.

Even if it's not containerized, you'd want to be only ever running it on a test system. If for some reason, you still can't provide full permissions, then it is ok for some tests to fail as it points out flaws in the CI setup. We need full permissions in CI and if we don't have it then it means it's not set up correctly.

I'm still not convinced that we want to modify this test file.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants