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

Standalone - Fix some path calculations #26649

Merged
merged 2 commits into from
Jun 26, 2023

Conversation

totten
Copy link
Member

@totten totten commented Jun 25, 2023

Overview

Fix a few edges in how paths/URLs are calculated

cc @artfulrobot

Before

  • Absolute URLs aren't generated. (They're always domain-relative.) Ex:
    • Run a command like cv url civicrm/dashboard
    • It should return an absolute URL (https://example.org/civicrm/dashboard) but actually returns relative (/civicrm/dashboard).
  • Some E2E test-runs fail because of trailing-slash confusion on the $root variable

After

  • Absolute URLs work.
  • Those E2E test are runnable.

@civibot
Copy link

civibot bot commented Jun 25, 2023

(Standard links)

@artfulrobot
Copy link
Contributor

artfulrobot commented Jun 26, 2023

URL: This looks good to me. I've r-run the cv command and get an absolute URL as desired.

Running the e2e tests fails though for me with phpunit8 --group e2e tests/phpunit/E2E

There were 2 errors:                                                                                                                                                                          
                                                                                                                                                                                              
1) E2E\Core\LocalizedDataTest::testLocalizedData                                                                                                                                              
RuntimeException: Failed to determine installation type for Standalone

/buildkit/build/standalonebk1/vendor/civicrm/civicrm-core/tests/phpunit/E2E/Core/LocalizedDataTest.php:65
/buildkit/build/standalonebk1/vendor/civicrm/civicrm-core/tests/phpunit/E2E/Core/LocalizedDataTest.php:24
/buildkit/.local/bin/phpunit8:1681

2) E2E_Extern_CliRunnerTest::testPermissionLookup
CRM_Core_Exception: Expected to find one User record, but there were zero.

/buildkit/build/standalonebk1/vendor/civicrm/civicrm-core/CRM/Utils/Array.php:1241
/buildkit/build/standalonebk1/vendor/civicrm/civicrm-core/Civi/Api4/Generic/Result.php:90
/buildkit/build/standalonebk1/vendor/civicrm/civicrm-core/ext/standaloneusers/Civi/Standalone/Security.php:92
/buildkit/build/standalonebk1/vendor/civicrm/civicrm-core/CRM/Utils/System/Standalone.php:235
/buildkit/build/standalonebk1/vendor/civicrm/civicrm-core/tests/phpunit/E2E/Extern/CliRunnerTest.php:59
/buildkit/.local/bin/phpunit8:1681

There were 20 failures:

...

It could be I'm not running it right?

(If the passing CI tests mean it works, then this is merge ready.)

@totten
Copy link
Member Author

totten commented Jun 26, 2023

@artfulrobot Glad that URLs worked.

(If the passing CI tests mean it works, then this is merge ready.)

Yeah, Jenkins passing basically means that everything runs on D7.

For Standalone E2E tests, I do see similar failures on the local - and that represents progress (from my POV). Before the patch, I had a hard-crash like this:

[bknix-min:~/bknix/build/s2/vendor/civicrm/civicrm-core] phpunit8 tests/phpunit/E2E/Core/
PHPUnit 8.5.27 #StandWithUkraine

......FFFE
Warning: require_once(/Users/totten/bknix/build/s2/web../vendor/autoload.php): failed to open stream: No such file or directory in /Users/totten/bknix/build/s2/vendor/civicrm/civicrm-core/CRM/Utils/System/Standalone.php on line 296

...(snip)...

    0.4503   62370928   7. CiviEndToEndTestCase::setUpBeforeClass() phar:///Users/totten/bknix/extern/phpunit8/phpunit8.phar/phpunit/Framework/TestSuite.php:436
    0.5210   63547808   8. CRM_Utils_System::loadBootStrap($params = ['name' => 'admin', 'pass' => 'admin'], $loadUser = ???, $throwError = ???, $realPath = ???) /Users/totten/bknix/build/s2/vendor/civicrm/civicrm-core/tests/phpunit/CiviTest/CiviEndToEndTestCase.php:18
    0.5210   63547808   9. CRM_Utils_System_Standalone->loadBootStrap($params = ['name' => 'admin', 'pass' => 'admin'], $loadUser = TRUE, $throwError = TRUE, $realPath = NULL) /Users/totten/bknix/build/s2/vendor/civicrm/civicrm-core/CRM/Utils/System.php:1544

After the patch, the test-suite runs to completion -- which means we can at least report on the failures. 😃 (For me, with pending patches, E2E currently gives ~45 errors/failures and ~1000 passes.)

@totten totten merged commit f50df3f into civicrm:master Jun 26, 2023
@totten totten deleted the master-standalone-paths branch June 26, 2023 20:01
@artfulrobot
Copy link
Contributor

Great thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants