Skip to content

Conversation

@schlndh
Copy link
Contributor

@schlndh schlndh commented Sep 18, 2022

I want to propose extracting the part of RuleTestCase::analyse that gathers the actual errors into its own public method so that the child test can do the comparison in a different way when necessary.

The use-case I have for this is that I want to use golden tests (i.e. gather the errors once, save them into a file, check them manually and then test against this saved sample in the future) instead of writing down the expected errors manually. This should make it possible to change the error messages easily, as well as edit the analysed file without having to worry about line numbers etc.

Unfortunately, the current interface of the RuleTestCase does not allow me to do this easily. For now, I'm using this workaround but as I'm relying on things not covered by BC promise, it could break easily in the future.

@herndlm
Copy link
Contributor

herndlm commented Sep 18, 2022

Interesting, fyi @matthiasnoback also recently started a discussion related to RuleTestCase: phpstan/phpstan#8006

@schlndh
Copy link
Contributor Author

schlndh commented Sep 18, 2022

@herndlm Thanks for the context. In fact I used something quite similar (at least as far as I can tell from having a brief look) before this. Here is my previous test case and an input file.

However, both of these approaches only solve the issue with line numbers (and only partially). I'd also want to avoid writing down the error messages myself. At the very least I'd want to call a parametrized method to generate the error message (so I can at least fix typos etc easily). However, generating the whole thing automatically and then just checking it seems even better (even if some hacks are required).

@ondrejmirtes ondrejmirtes merged commit 2222b63 into phpstan:1.8.x Oct 17, 2022
@ondrejmirtes
Copy link
Member

Thank you.

@schlndh schlndh deleted the feature-ruleTestCaseGatherAnalyserErrors branch October 17, 2022 12:53
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