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

feat(ECCW-509): DDEV commands, phpcs*.xml, phpunit*.xml. #465

Merged
merged 9 commits into from
Sep 2, 2024

Conversation

Polynya
Copy link
Collaborator

@Polynya Polynya commented Jul 26, 2024

Include a summary of what this merge request involves (*)

This is the first of a series of tickets to improve testing.

Based on files in localgov profile.
DDEV commands:

  • phpcs - check PHP coding standards on custom modules and ecc_theme themes
  • phpcbf - fix PHP coding standards on custom modules and ecc_theme themes
  • phpunit - run PHPUnit tests on custom modules
  • phpunit-lgd - run PHPUnit tests on LGD modules, profile and theme

XML files:

  • phpcs-ecc.xml - ruleset for PHP coding standards on custom modules and ecc_theme themes
  • phpcs-lgd.xml - ruleset for PHP coding standards on LGD modules, profile and theme
  • phpunit-ecc.xml - PHPUnit configuration for testing custom modules and ecc_theme themes
  • phpunit-lgd.xml - PHPUnit configuration for testing LGD modules, profile and theme

Call out any relevant implementation decisions

  • Are there any links to back up your chosen methodology?

  • Why have you taken the approach?
    PHPCS reports issues in ecc_theme themes but PHPCBF does not fix them. This is to ensure that the developer fixes and commits the changes to the ecc_theme repo.

  • Any particular problem areas?

If necessary, please include any relevant screenshots (If not already available on the JIRA ticket)

This PR has been tested in the following browsers

  • Arc
  • Edge
  • Chrome
  • Safari
  • Firefox
  • IE 11 (Windows)
  • iOS Chrome
  • iOS Safari
  • Android Chrome
  • Android Firefox
  • Android default

@Polynya Polynya requested a review from siliconmeadow July 26, 2024 16:47
Copy link
Member

Choose a reason for hiding this comment

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

@Polynya - Do you suppose the Failed to run msg is due to the items it couldn't fix?
image

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, I think so. The error message is a bit confusing. Maybe we need a script to check the error code

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The error is caught in the script and a friendly message displayed
image

Copy link
Member

Choose a reason for hiding this comment

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

I just tried the two phpunit commands as well and the php-lgd also shows the confusing message:

image

The standalone phpunit didn't, but that could be due to no tests were run:

image

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Thanks for testing. For me, phpunit-lgd is getting stuck on the FunctionalJavascript tests. When I ran it a few weeks ago it got to the end and reported all the deprecations. For the ECC tests I added this variable
export SYMFONY_DEPRECATIONS_HELPER=weak
I've added an extra command without this - ddev phpunit-with-deprecations

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I'm going to add a test to a module in the next PR when I add testing to the pipeline.

@Polynya Polynya merged commit 33c61ed into develop Sep 2, 2024
Copy link

github-actions bot commented Sep 2, 2024

Stopping Review App and clearing up

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