Skip to content

Don't skip tests which are supposed to fail; mark them as xfail #15472

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
merged 1 commit into from
Aug 18, 2024

Conversation

cmb69
Copy link
Member

@cmb69 cmb69 commented Aug 17, 2024

Especially regarding buggy server behavior, we should not skip those tests, because it is unlikely that fixes to the server's behavior will even be noticed. Instead we mark these tests as xfail, so we get a warning if the test succeeds, and can act appropriately.


Note that mysqli_stmt_attr_get_prefetch.phpt passes for me with MySQL 5.6, 8.0 and 9.0.

Especially regarding buggy server behavior, we should not skip those
tests, because it is unlikely that fixes to the server's behavior will
even be noticed.  Instead we mark these tests as xfail, so we get a
warning if the test succeeds, and can act appropriately.
@cmb69
Copy link
Member Author

cmb69 commented Aug 17, 2024

Note that mysqli_stmt_attr_get_prefetch.phpt passes for me with MySQL 5.6, 8.0 and 9.0.

As well as on some of the CI workflows. That should be investigated; not sure if that test is executed on the other workflows.

Copy link
Member

@kamil-tekiela kamil-tekiela left a comment

Choose a reason for hiding this comment

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

LGTM assuming the tests actually do what they do. As a follow-up, I will look into this.

@kamil-tekiela kamil-tekiela merged commit ed2b456 into php:master Aug 18, 2024
10 checks passed
@cmb69 cmb69 deleted the cmb/skip-to-xfail branch August 18, 2024 12:03
@kamil-tekiela
Copy link
Member

Note that mysqli_stmt_attr_get_prefetch.phpt passes for me with MySQL 5.6, 8.0 and 9.0.

See How to use mysqli_stmt_attr_get() and mysqli_stmt_attr_set() functions in php?

The XFAIL section for this test can be removed. It's a pretty useless test though. The value will always be 1, so it essentially duplicates mysqli_stmt_attr_get.phpt. Maybe we should drop this test. What do you think?

@kamil-tekiela
Copy link
Member

I have a better alternative #15485

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