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

Check "Status Report" page #398

Closed
mrdavidburns opened this issue Feb 5, 2024 · 10 comments · Fixed by #684
Closed

Check "Status Report" page #398

mrdavidburns opened this issue Feb 5, 2024 · 10 comments · Fixed by #684
Assignees
Labels
enhancement New feature or request

Comments

@mrdavidburns
Copy link
Member

For all pull requests and deployments we check the /admin/reports/status page to make sure we're not introducing new things to be fixed.

For example: we may start a project where there are 5 warnings of things that should be fixed. We want to make sure we're not introducing more warnings as we maintain the site. If anything we want to reduce the number of warnings.

@YesCT YesCT added the enhancement New feature or request label Feb 5, 2024
@YesCT
Copy link
Contributor

YesCT commented Feb 5, 2024

This could also be an e2e test that runs using the CI using same ddev build as other e2e tests.

@deviantintegral
Copy link
Member

I think this is probably easiest and fastest using drush's JSON output, rather than browsing and parsing the status report HTML page.

@beto-aveiga
Copy link
Collaborator

For example: we may start a project where there are 5 warnings of things that should be fixed.

I'm wondering where would we store this initial/base number.
We can check for errors with Drush as @deviantintegral mentioned, but how could we store that number?

@deviantintegral
Copy link
Member

I think rather than storing a count, we could have a known-issues list that's committed to the repo, like we've done with security issues awaiting patches.

@beto-aveiga
Copy link
Collaborator

like we've done with security issues awaiting patches.

@deviantintegral is this in the Drainpipe's repo?

Because this is constrained to Tugboat, we may not need to store the "Status Report" result in our repo.
I imagine we would have a GitHub action to do something like the following:

  • Generate our own drush status-report --format json (store it at /sites/default/files/drainpipe/status-report.json)
  • Get the base preview URL or the default preview URL
  • Get the base/default preview file at /sites/default/files/drainpipe/status-report.json
  • Compare both Status Report files and fail PR check if there is a new record with severity "Warning" or "Error" (the threshold for failing the check could be adjusted in composer.json)
  • Adding or not this Github action can be done through Composer settings (to install it when running composer install).

Does it make sense?

@deviantintegral
Copy link
Member

like we've done with security issues awaiting patches.
@deviantintegral is this in the Drainpipe's repo?

I don't think it is! @justafish do you think that makes sense to add?

Also, your thoughts on @beto-aveiga 's plan would be good too.

@justafish
Copy link
Member

It's pending this issue: #112

I don't think it should be constrained to Tugboat - you should be able to provide any URL to test

@justafish justafish changed the title Check "Status Report" page on Tugboat builds Check "Status Report" page Jun 17, 2024
@beto-aveiga
Copy link
Collaborator

I think this test should be just a script containing a drush command to check for no errors or warnings on the status report page.
We should add this script at /test/phpunit when running composer install if DRUPAL_TEST_TRAITS is TRUE... or similar, because we need a Drupal working site.

I don't have any LSM projects referencing DRUPAL_TEST_TRAITS, except for Lullabot, which is FALSE with a comment above: "Disable DTT tests until they are fixed."

@mrdavidburns
Copy link
Member Author

mrdavidburns commented Sep 5, 2024

see: drush core:requirements --severity=2

  • Add to the ddev task build:dev command.

@beto-aveiga
Copy link
Collaborator

Just to let you know this has been solved here:
#684

And the PR is already approved.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants