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

Fix IntrospectionProcessor tests resulting in false positives #1896

Merged
merged 1 commit into from
Jun 28, 2024

Conversation

healsdata
Copy link
Contributor

Three tests in IntrospectionProcessorTest (testLevelTooLow, testLevelEqual, testLevelHigher) aren't actually testing anything. Because $expected = $input is a reference, the changes made to $expected['extra'] are made to $input and carried forward to $actual. You can demonstrate this by adding a return $record at the immediate start of InstrospectionProcessor::__invoke -- the tests still pass despite bypassing all the code.

Fixes #1895

…der test

Three tests in IntrospectionProcessorTest (testLevelTooLow, testLevelEqual, testLevelHigher) aren't actually testing anything. Because `$expected = $input` is a reference, the changes made to `$expected['extra']` are made to $input and carried forward to $actual. You can demonstrate this by adding a `return $record` at the immediate start of `InstrospectionProcessor::__invoke` -- the tests still pass despite bypassing all the code.
@healsdata healsdata changed the title Fix IntrospectionProcessor tests with incorrect setup. Fix IntrospectionProcessor tests resulting in false positives Jun 19, 2024
@Seldaek
Copy link
Owner

Seldaek commented Jun 28, 2024

Thanks, makes sense! This is because it was previously just using arrays and it didn't get migrated correctly in v3.

@Seldaek Seldaek merged commit 4e03d25 into Seldaek:main Jun 28, 2024
7 of 8 checks passed
@Seldaek Seldaek added this to the 3.x milestone Jun 28, 2024
@healsdata healsdata deleted the fix-introspection-tests branch June 29, 2024 01:20
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.

IntrospectionProcessorTests: $expect and $actual are the same object by reference.
2 participants