Skip to content

Correct the deep copy and testing system - #22

Merged
stefanheimes merged 4 commits into
masterfrom
hotfix/deep-copy-support
Jan 28, 2026
Merged

Correct the deep copy and testing system#22
stefanheimes merged 4 commits into
masterfrom
hotfix/deep-copy-support

Conversation

@stefanheimes

@stefanheimes stefanheimes commented Jan 26, 2026

Copy link
Copy Markdown
Member

Description

Please explain the detailed changes you made here.
Reference any issue number this pull request fixes.

Checklist

  • Read and understood the CONTRIBUTING guidelines
  • Created tests, if possible
  • All tests passing
  • Extended the README / documentation, if necessary
  • Added myself to the @authors in touched PHP files
  • Checked the changes with phpcq and introduced no new issues
@zonky2
zonky2 requested a review from discordier January 26, 2026 14:07
@zonky2 zonky2 added the bug Something isn't working label Jan 26, 2026
@zonky2
zonky2 self-requested a review January 26, 2026 14:07
@zonky2 zonky2 added this to the 2.3.* milestone Jan 26, 2026

@zonky2 zonky2 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

$widget
->expects(self::any())
->method('import')
->withConsecutive([Config::class, 'Config']);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

shouldn't this whole block rathe be:

        $widget
            ->expects($this->once())
            ->method('import')
            ->with(Config::class, 'Config');

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

@discordier Same here. The import is never called.

Comment thread .phpcq.yaml.dist
trusted-keys:
# composer-require-checker
- 033E5F8D801A2F8D
- B2BDAAAC6F1FDE528CD9EEC9033E5F8D801A2F8D

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

While these are not wrong, they point to a bug in phpcq... Apparently the cli of gnupg uses a different key fingerprint hash len than the php pgp extension.

We should fix this upstream and then remove them here again.

@stefanheimes

Copy link
Copy Markdown
Member Author

@discordier I've made a few updates. However, the "import" call has been removed from the phpunit section. Could you please take another look at it via the pull request?

$this->assertInstanceof(Config::class, $parameters[0]);
$this->assertSame('Config', $parameters[1]);
});
->with(Config::class, 'Config');

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

When using never, "with" doesn't make sense at all.

@stefanheimes
stefanheimes merged commit 17bd28d into master Jan 28, 2026
4 checks passed
@discordier
discordier deleted the hotfix/deep-copy-support branch February 2, 2026 14:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

3 participants