search

phpactor / phpactor

Mainly a PHP Language Server with more features than you can shake a stick at
MIT License
1.45k stars 131 forks source link

Project: allow to store test cases describing known bugs #2198

Closed przepompownia closed 1 year ago

przepompownia commented 1 year ago

For example, consider #2195. It contains a data set provided for IndexedReferenceFinderTest::testFinder with expected value for which the test fails before resolving #2195.

We could store such tests in the project and treat them in a special way. I don't know any widely known good practice to do it (or if the whole proposition is a known anti-pattern).

  1. probably we would need to use some test group, e.g. bugs to exclude it from the default CI job,
  2. probably we should run such test group separately to control the current number of failed cases (and don't forget to change some test that stopped failing),
  3. a strange alternative to 2.: assertions are negations of expected values: after fixing some bug such its corresponding test starts failing with some appropriate message, e.g. assertNotTrue($expectedTrueValue, 'Check if #2195 was fixed and possibly convert this test.').

What do you think about storing such cases (@dantleech especially)?

mamazu commented 1 year ago

I totally agree that tracking what fails and what doesn't in terms of bugs can really be helpful. Technically it would be possible to add another PhpUnit test suite with only bug tests like you described in number 3.

dantleech commented 1 year ago

Not sure this makes sense given that these would still need to be tracked by issues and we can just as well either raise a PR with the failing test or include a failing test case in the issue itself. Having tests which are ignored on CI means they can be forgotten, or can otherwise rot..

mamazu commented 1 year ago

I think the only thing this protects against is accidentally fixed test and their regression. But I don't think that this is that important, if you want to tackle an issue just copy over the test in the ticket I guess.

przepompownia commented 1 year ago

I do not mean to ignore such tests by CI but for example by its default job (phpunit) - rather to run them in a separated job assuming either tracking the number of failures or defining them with negated assertions and messages.

Storing them simply in PR branches also does not seem the worst approach.

dantleech commented 1 year ago

Closing as I think we can do this more effectively with PRs

przepompownia commented 1 year ago

@dantleech @mamazu thanks for submitting your opinion!

dantleech commented 1 year ago

and feel free to discuss further if required, just trying to combat the issue queue 😄