doctrine-yuml-bundle icon indicating copy to clipboard operation
doctrine-yuml-bundle copied to clipboard

Fixed compatibility with Symfony 5

Open Steveb-p opened this issue 4 years ago • 8 comments

This PR is a follow up for #12.

Also adds Github Action configuration for automatic tests.

It provides the following changes:

  • Fixes base issues with tests (I need to revisit them once more to provide more sane defaults for config arrays in actual implementations)
  • Adds missing symfony/yaml dependency (we could also try moving over to PHP/XML service declarations)
  • Fixes Command reliance on Container. Now dependencies are injected via constructor, just like in Controller.
  • Adds a virtual file system for one of the tests.
  • Adds missing ext-curl dev dependency.
  • Fixed DataCollector serialization tests.
  • Fixed lowest dependency versions (issues detected when installing composer update --prefer-lowest, and testing)

Steveb-p avatar Mar 19 '21 08:03 Steveb-p

seems already great ! Are all tests OK for you or do you need more time ? (don't have much time left actually to test by myself) tell me when (or if) you think it is Ok for you :) Thanks a lot for your contribution, it helps a lot !

Nono1971 avatar Mar 24 '21 20:03 Nono1971

@Nono1971 I'll look at it again tonight. I had a few thoughts at the end that I wanted to look into.

Are you ok with switching to GH actions, or should I put into a second PR? (They need to be enabled in repository settings too).

Steveb-p avatar Mar 25 '21 08:03 Steveb-p

OK, I let you see tonight :)

For GH actions, I'm neither OK or KO, I just didn't think about it yet. If you have propositions about it, why not ! (don't know if it would be worth or not yet... but it might be interesting to look at it). Anyway, it would effectively be better into another PR :) Maybe could you open an issue for this evolution to talk together about the possibilities before working on it ^-^

Nono1971 avatar Mar 25 '21 09:03 Nono1971

Sorry, didn't manage to pick it up yesterday evening. Will try to today.

Regarding GH actions I'll move it to a separate PR. I'm pretty sure it's something you'd like to have since it uses the Github infrastructure (for open source), can report directly into the PR and since it works on containers it's faster (Travis spins up virtual machines for each test, and they recently introduced some limitations into their free, OS version afaik).

Steveb-p avatar Mar 26 '21 08:03 Steveb-p

@Nono1971 Github actions are removed via 4027aa8.

I've up'ed the PHPUnit version requirement to ^8.0 for future PHP 8.0 compatibility / tests. Symfony 4.x tests will fail against PHP 8 atm.

Added more PHP & Symfony versions to testing matrix.

Code style sniffer version has been up'ed to 3.4. Previous versions fail on PHP 7.4.

Lastly, I've added a Kernel integration test that ensures that the DI container will properly compile in a smallest possible application. Unfortunately due to issues with a warning during compilation:

There was 1 error:

1) OnurbTest\Bundle\YumlBundle\KernelTest::testKernelCompiles
"continue" targeting switch is equivalent to "break". Did you mean to use "continue 2"?

I mark the test as skipped for Symfony 4.x.

Steveb-p avatar Mar 27 '21 12:03 Steveb-p

Fixed issues shown in tests. Now they pass properly: https://travis-ci.org/github/Nono1971/doctrine-yuml-bundle/builds/764642437

Ideally PR's with failing tests should not be allowed to be merged I guess, but that goes into repository configuration and travis integration :smile:

Steveb-p avatar Mar 27 '21 13:03 Steveb-p

???

andrelec1 avatar Jul 05 '21 13:07 andrelec1

Up!!

ahmed-bhs avatar Jul 06 '22 15:07 ahmed-bhs