Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bugfix: Continuous integration should be properly executed #57

Closed
wants to merge 3 commits into from

Conversation

boesing
Copy link
Member

@boesing boesing commented Oct 10, 2021

Q A
Bugfix yes
QA yes

Description

This will ensure that the CI pipeline for this project will run as expected. This also shows that Auryn container (as mentioned in #53) is not compatible with mezzio/mezzio (anymore).

This PR also contains the following changes:

  • Removal of phpstan/phpstan
  • Implementation for vimeo/psalm
  • Removal of phpspec/prophecy
  • Removal of composer.lock as part of the asset removal

@boesing boesing added the Bug Something isn't working label Oct 10, 2021
@boesing boesing added this to the 3.11.0 milestone Oct 10, 2021
@boesing boesing changed the title Bugfix: Continious integration should be properly executed Bugfix: Continous integration should be properly executed Oct 10, 2021
@boesing boesing changed the title Bugfix: Continous integration should be properly executed Bugfix: Continuous integration should be properly executed Oct 10, 2021
@boesing boesing marked this pull request as draft October 10, 2021 23:18
@boesing boesing force-pushed the bugfix/continious-integration branch 2 times, most recently from 15fd098 to 4520991 Compare October 10, 2021 23:38
This will ensure that we do not end up having the `MezzioInstaller` executed while running CI.

Signed-off-by: Maximilian Bösing <2189546+boesing@users.noreply.github.com>
This also replaces `phpstan/phpstan` with `vimeo/psalm`.

Signed-off-by: Maximilian Bösing <2189546+boesing@users.noreply.github.com>
Signed-off-by: Maximilian Bösing <2189546+boesing@users.noreply.github.com>
@weierophinney weierophinney force-pushed the bugfix/continious-integration branch from 4520991 to 2047d07 Compare December 22, 2021 17:53
@weierophinney
Copy link
Contributor

@boesing I tried to resolve the errors that were occuring in your CI runs, got confused, and decided to start from scratch. I started by removing prophecy and switching to native PHP mock objects, following your examples as closely as possible.

What I ended up with was that the TemplateRenderersTest and the HomePageResponseTest had failures, and I was utterly unable to resolve them.

I started by getting errors about declare(strict_types=1) not being the first line of vendor/phpunit/phpunit/phpunit. Doing some sleuthing, it appears this happens when you call include() on a file that has a shabang line. When was that happening? I realized it was due to these tests having the @runInSeparateProcess annotation; for some reason, the autoloader was trying to load the vendor/phpunit/phpunit/phpunit during subsequent processes, which raised this error. (I think this is likely similar to the file_get_contents('./composer.json') file not found error you're seeing.)

So, I removed the annotation... and the first run for a given template renderer case would pass, but then it would start failing, generally for all other cases.

From what I could tell, it was an issue due to caching of the App\ConfigProvider class. Since this class is used to determine the paths to template files, the following would happen:

  • Since the Laminas view renderer tests would run first, when it looked for Twig-related files, it was looking relative to the original installation directory in which we were testing Laminas view renderer. That directory would no longer exist (it was removed after those tests), so it could not find the files. When I commented out the logic that would cleanup after each run, it was finding the Laminas template files, but not finding the Twig files (as they were in a different installation directory).
  • Since the Plates templates share the same suffix, I'd either get a file not found error (if I was cleaning up after each run) or I'd get errors with rendering due to unknown template methods and/or variables, because it was trying to use Laminas templates instead of Plates templates.

I can run each of those tests individually, and they pass. Running them as part of the test suite, they fail (due to cached class declarations). Running them in isolated processes fails (due to oddities with autoloading).

I'm not sure why this changes simply by switching from prophecy to native mocks. If I could resolve the @runInSeparateProcess issue, I think these would run fine, and I could finish up. For now, though, I'm stuck. ☹️

@weierophinney weierophinney removed this from the 3.11.0 milestone Jan 3, 2022
@weierophinney
Copy link
Contributor

Closing in favor of #63 - I managed to get things sorted!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants