Skip to content

Use json_decode to produce a non-interned string in typeMap test#1985

Merged
GromNaN merged 1 commit intomongodb:v2.xfrom
GromNaN:phpc-1839-opcache-skipif
Apr 15, 2026
Merged

Use json_decode to produce a non-interned string in typeMap test#1985
GromNaN merged 1 commit intomongodb:v2.xfrom
GromNaN:phpc-1839-opcache-skipif

Conversation

@GromNaN
Copy link
Copy Markdown
Member

@GromNaN GromNaN commented Apr 13, 2026

Replace chr(ord('a')).'rray' with json_decode('"array"') to produce a non-interned string that opcache cannot fold, allowing the test to run under all configurations.

@GromNaN GromNaN requested a review from a team as a code owner April 13, 2026 17:15
@GromNaN GromNaN requested review from alcaeus and Copilot and removed request for a team April 13, 2026 17:15
@GromNaN GromNaN changed the title PHPC-1839: Skip test when opcache.enable_cli is enabled Skip test when opcache.enable_cli is enabled Apr 13, 2026
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR updates the PHPT test suite to avoid a known failure mode when OPcache is enabled for CLI, which can change string interning behavior and invalidate the assumptions of an existing BSON test.

Changes:

  • Add a skip_if_opcache_enabled_cli() helper to centralize OPcache CLI skip logic.
  • Update bug1839-005.phpt to skip when opcache.enable_cli=1.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
tests/utils/skipif.php Adds a reusable skip helper for OPcache CLI affecting string interning/constant folding assumptions.
tests/bson/bug1839-005.phpt Uses the new skip helper via basic-skipif.inc to prevent false failures when OPcache CLI is enabled.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread tests/bson/bug1839-005.phpt Outdated
@GromNaN GromNaN force-pushed the phpc-1839-opcache-skipif branch from 9d3f835 to 160c759 Compare April 15, 2026 09:59
@GromNaN GromNaN changed the title Skip test when opcache.enable_cli is enabled PHPC-1839: Use json_decode to produce a non-interned string in typeMap test Apr 15, 2026
@GromNaN
Copy link
Copy Markdown
Member Author

GromNaN commented Apr 15, 2026

Updated: replaced chr(ord('a')).'rray' with json_decode('"array"') as suggested. No need to skip the test anymore.

@GromNaN GromNaN force-pushed the phpc-1839-opcache-skipif branch from 160c759 to 28cc403 Compare April 15, 2026 10:01
Copilot AI review requested due to automatic review settings April 15, 2026 10:01
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Updates a PHPT regression test for PHPC-1839 to reliably generate a non-interned "array" string at runtime, avoiding OPcache constant-folding so the test behaves consistently across configurations.

Changes:

  • Replace chr(ord('a')).'rray' string assembly with json_decode('"array"') to ensure a non-interned runtime string.
  • Keep typemap references (&$rootValue, &$documentValue) pointing at non-interned strings for the test scenario.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@GromNaN GromNaN enabled auto-merge (squash) April 15, 2026 10:06
@GromNaN GromNaN changed the title PHPC-1839: Use json_decode to produce a non-interned string in typeMap test Use json_decode to produce a non-interned string in typeMap test Apr 15, 2026
@GromNaN GromNaN disabled auto-merge April 15, 2026 10:07
@GromNaN GromNaN enabled auto-merge (squash) April 15, 2026 10:07
Comment thread tests/bson/bug1839-005.phpt
@GromNaN GromNaN disabled auto-merge April 15, 2026 11:45
@GromNaN GromNaN force-pushed the phpc-1839-opcache-skipif branch from 28cc403 to a463086 Compare April 15, 2026 11:55
Copy link
Copy Markdown
Member

@alcaeus alcaeus left a comment

Choose a reason for hiding this comment

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

LGTM

@GromNaN GromNaN enabled auto-merge (squash) April 15, 2026 12:02
@GromNaN GromNaN merged commit fc6e9b3 into mongodb:v2.x Apr 15, 2026
33 of 43 checks passed
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