GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — Assertion class as parameter for Codeception config by Slamdunk · Pull Request #65 · phpstan/phpstan-phpunit

Conversation

@Slamdunk

@Slamdunk Slamdunk commented Mar 20, 2020

Copy link
Copy Markdown

Resolves #34

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
@Slamdunk
Make assertion types class a parameter
c2975d7
Slamdunk
Slamdunk commented Mar 20, 2020
View reviewed changes
Comment thread extension.neon
-
class: PHPStan\Type\PHPUnit\Assert\AssertMethodTypeSpecifyingExtension
arguments:
classWithAssertionMethods: PHPUnit\Framework\TestCase

@Slamdunk Slamdunk Mar 20, 2020 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

I've retained the class used in https://github.com/phpstan/phpstan-phpunit/pull/65/files#diff-afc1ebcbfd95543266d01a244f0e43deL27, but shouldn't this be PHPUnit\Framework\Assert too?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

nreynis reacted with thumbs up emoji
All reactions
  • 👍 1 reaction
@Slamdunk
Add README how-to for Codeception
5af462c
@Slamdunk
Slamdunk force-pushed the parameters_for_codeception branch from bda246e to 5af462c Compare March 20, 2020 08:12
@ondrejmirtes

ondrejmirtes commented Mar 20, 2020

Copy link
Copy Markdown
Member

Hi, I don't know anything about Codeception. Do you typically use assert* methods from PHPUnit in conjunction with Codeception methods, or do you use either PHPUnit or Codeception?

Anyway, we could register everything by default (there's no downside) and just mention in the README that this extension works well with Codeception out of the box. WDYT?

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@Slamdunk

Slamdunk commented Mar 20, 2020

Copy link
Copy Markdown
Author

It was my wish from the beginning, but it not feasible.

Codeception ships with 2 different types of test, the Unit one which is a subclass of PHPUnit\Framework\TestCase and already works out of the box, and the Functional/Acceptance ones. In a nutshell, Functional/Acceptance tests gather multiple classes and their methods, copy-paste their signatures into a giant class, and proxy all the calls to the original ones. This allows the user to choose which module to use in the tests (i.e. Filesystem, Redis, Symfony, FTP...) and call all the available assertions/helper-methods under the $this hat. The Assertion module is one of them and is a proxy to the PHPUnit one.

What's the blocking? The giant class created lies under the project folder tree, and of course to be correctly autoloaded it must have the project's namespace, not a generic one.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@nreynis

nreynis commented Sep 30, 2021 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown

I feel like this is related to #103

I don't exactly see the benefit to parametrize the assertion class instead of just fixing the FQCN. But I'm not familiar at all with codeception. Can you provide a link to that Acceptance class (or other relevant code) is it extending Assert?

I tried searching https://github.com/Codeception/phpunit-wrapper/tree/9.0/src but I didn't find it.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@Slamdunk

Slamdunk commented Sep 30, 2021

Copy link
Copy Markdown
Author

Can you provide a link to that Acceptance class (or other relevant code) is it extending Assert?

I doesn't exist, Codeception doesn't work this way.
It generates Traits that proxy the calls to PHPUnit's Assert class, and it generates them dynamically based on how the developer configured Codeception.

There is no way phpstan/phpstan-phpunit can fix #34 with an hardcoded code change.

nreynis reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@nreynis

nreynis commented Oct 2, 2021

Copy link
Copy Markdown

The thing is making the class parametrizable will fix your use case by breaking PHP Unit analysis.

I think a better approach would be to create another package dedicated to codeception, dependant on this one. This way you can have two instance of the extension working at the same time.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@Slamdunk

Slamdunk commented Oct 4, 2021

Copy link
Copy Markdown
Author

The thing is making the class parametrizable will fix your use case by breaking PHP Unit analysis.

May I ask you how this break would happen?

I think a better approach would be to create another package dedicated to codeception, dependant on this one. This way you can have two instance of the extension working at the same time.

This PR already enable to have both PHPUnit and Codeception working in the same time, where do you see this isn't the case?

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@nreynis

nreynis commented Oct 4, 2021

Copy link
Copy Markdown

🤦 You are right I didn't understand the PR correctly. I somehow thought you were just feeding the existing service with another default value. But you are in fact creating other instances with different parameters.

This should work fine. 👍

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

assertInternalType()

Uh oh!

There was an error while loading. Please reload this page.

3 participants

@Slamdunk @ondrejmirtes @nreynis
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.