Skip to content

ext/mysqli: Skip test that needs superuser#17531

Open
NattyNarwhal wants to merge 4 commits into
php:masterfrom
NattyNarwhal:mysqli-skip-unprivileged
Open

ext/mysqli: Skip test that needs superuser#17531
NattyNarwhal wants to merge 4 commits into
php:masterfrom
NattyNarwhal:mysqli-skip-unprivileged

Conversation

@NattyNarwhal

Copy link
Copy Markdown
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
Copy Markdown
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?

Comment thread ext/mysqli/tests/mysqli_get_client_stats.phpt Outdated
Comment thread ext/mysqli/tests/mysqli_get_client_stats.phpt Outdated
Comment thread ext/mysqli/tests/mysqli_get_client_stats.phpt Outdated
@NattyNarwhal

Copy link
Copy Markdown
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.

NattyNarwhal and others added 3 commits January 20, 2025 12:35
Co-authored-by: Kamil Tekiela <tekiela246@gmail.com>
Co-authored-by: Kamil Tekiela <tekiela246@gmail.com>
@kamil-tekiela

Copy link
Copy Markdown
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