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

Fix E2E admin login failure #2689

Closed

Conversation

mikkamp
Copy link
Contributor

@mikkamp mikkamp commented Nov 22, 2024

Created this draft PR to test E2E login on admin page. Not able to reproduce this failure locally.

Added screenshot recording to facilitate troubleshooting, this is what the login screen looks like:

admin-page-0

@mikkamp mikkamp self-assigned this Nov 22, 2024
@mikkamp mikkamp added the changelog: none Skip changelog entry for this PR label Nov 22, 2024
Copy link

codecov bot commented Nov 22, 2024

Codecov Report

Attention: Patch coverage is 0% with 1 line in your changes missing coverage. Please review.

Project coverage is 65.0%. Comparing base (ed1bc62) to head (9065bd4).

Files with missing lines Patch % Lines
...ernal/DependencyManagement/CoreServiceProvider.php 0.0% 1 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@                        Coverage Diff                         @@
##             fix/connect-to-invalid-ads-id   #2689      +/-   ##
==================================================================
+ Coverage                             62.3%   65.0%    +2.8%     
- Complexity                               0    4656    +4656     
==================================================================
  Files                                  335     812     +477     
  Lines                                 5145   24573   +19428     
  Branches                              1254    1254              
==================================================================
+ Hits                                  3204   15984   +12780     
- Misses                                1769    8417    +6648     
  Partials                               172     172              
Flag Coverage Δ
js-unit-tests 62.3% <ø> (ø)
php-unit-tests 65.8% <0.0%> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
...ernal/DependencyManagement/CoreServiceProvider.php 0.0% <0.0%> (ø)

... and 476 files with indirect coverage changes

---- 🚨 Try these New Features:

@mikkamp
Copy link
Contributor Author

mikkamp commented Nov 22, 2024

After using a filter to output the PHP error message, I uncovered the following:

admin-page-0

We check return defined( 'WP_CLI' ) && WP_CLI, but that's not the same as checking if the class is available.

@mikkamp
Copy link
Contributor Author

mikkamp commented Nov 22, 2024

Just to conclude, we found the culprit was the is_needed function was not running to check the conditions.

The following commit fixes this issue: c0fdbad

Closing this PR, but it has some useful commits to help surface errors in the E2E environment:

  • Manually take a screenshot (if placed in the right folder it will be part of the artifacts): 0669161
  • Use a filter to output the fatal error from WordPress within the screenshot (couldn't find a way to get direct access to the PHP log file): 952283b

@mikkamp mikkamp closed this Nov 22, 2024
@mikkamp mikkamp deleted the test/fix-e2e-admin-login-failure branch November 22, 2024 16:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
changelog: none Skip changelog entry for this PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant