Tdr 22/get depencencies from composer #764

hutnikau · 2020-04-07T08:07:25Z

No description provided.

common/ext/class.ExtensionsManager.php

common/oatbox/extension/ComposerInfo.php

helpers/class.ExtensionHelper.php

test/unit/ManifestTest.php

ks16

Some suggestions/ideas and failed tests.

common/oatbox/extension/ComposerInfo.php

ks16 · 2020-04-20T16:11:50Z

common/oatbox/extension/Manifest.php

+     * @throws ManifestNotFoundException
+     * @throws exception\MalformedManifestException
+     */
+    public function __construct($filePath, ComposerInfo $composerInfo = null)


I think it would be better to pass directory name instead of manifest file path here. This class is more suitable for manifest fine name constant, than common_ext_Extension and later in the code we use directory name to read composer files.

helpers/class.ExtensionHelper.php

scripts/update/Updater.php

test/unit/extension/ComposerInfoTest.php

…22/get_depencencies_from_composer

gitromba

composer.lock is meant to be for internal usage for Composer and its structure can vary from release to release.
Instead it's strongly advised to build on Composer's public API which has been introduced with Composer 2 and being supported by TAO.

@hutnikau now that we have Composer 2 support, please consider updating the PR to be built on a standard and reliable API.

…22/get_depencencies_from_composer

hutnikau · 2021-01-12T14:56:01Z

composer.lock is meant to be for internal usage for Composer and its structure can vary from release to release.
Instead it's strongly advised to build on Composer's public API which has been introduced with Composer 2 and being supported by TAO.

@hutnikau now that we have Composer 2 support, please consider updating the PR to be built on a standard and reliable API.

I pushed a commit with usage of composer API, but unfortunately it does not provide full information about the package, so we still need to read lock file (to get extra data from there).

…22/get_depencencies_from_composer

gitromba · 2021-01-14T20:02:16Z

I'm having failing unit tests locally:

❯ vendor/bin/phpunit generis/test/unit/extension/ComposerInfoTest.php
PHPUnit 8.5.13 by Sebastian Bergmann and contributors.

.E..                                                                4 / 4 (100%)

Time: 35 ms, Memory: 4.00 MB

There was 1 error:

1) oat\generis\test\unit\extension\ComposerInfoTest::testGetComposerLock
oat\oatbox\extension\exception\ManifestException: /Users/mihaly.hadju/repos/sandbox/generis/test/samples/manifests/composer.lock file not found

/Users/mihaly.hadju/repos/sandbox/generis/common/oatbox/extension/ComposerInfo.php:80
/Users/mihaly.hadju/repos/sandbox/generis/common/oatbox/extension/ComposerInfo.php:67
/Users/mihaly.hadju/repos/sandbox/generis/test/unit/extension/ComposerInfoTest.php:48

ERRORS!
Tests: 4, Assertions: 5, Errors: 1.

gitromba

Thanks for the updates @hutnikau. This is an important milestone in the life of TAO :).

There are some comments left. Most of them are originated to ComposerInfo and its contract. Please check them out!

common/ext/class.ExtensionsManager.php

common/oatbox/extension/ComposerInfo.php

common/oatbox/extension/Manifest.php

composer.json

gitromba · 2021-01-14T21:31:02Z

FYI @hutnikau: The corresponding PR CI build is tested with Composer 2. Looks okay, but there are broken tests .

…22/get_depencencies_from_composer

gitromba

Thanks for the quick update @hutnikau!

I have a failing unit test, please have a look:

❯ vendor/bin/phpunit generis/test/unit --bootstrap generis/test/bootstrap.php
PHPUnit 8.5.14 by Sebastian Bergmann and contributors.

...............................................................  63 / 308 ( 20%)
SSSSSSSS....................................................... 126 / 308 ( 40%)
..................SSSSSSSSSSSS................................. 189 / 308 ( 61%)
....................EF......................................... 252 / 308 ( 81%)
........................................................        308 / 308 (100%)

Time: 12.69 seconds, Memory: 24.00 MB

There was 1 error:

1) oat\generis\test\unit\extension\ComposerInfoTest::testGetAvailableTaoExtensions
common_ext_ExtensionException: Composer lock file missed at /composer.lock

/Users/mihaly.hadju/repos/sandbox/generis/common/oatbox/extension/ComposerInfo.php:156
/Users/mihaly.hadju/repos/sandbox/generis/common/oatbox/extension/ComposerInfo.php:78
/Users/mihaly.hadju/repos/sandbox/generis/test/unit/extension/ComposerInfoTest.php:43

--

There was 1 failure:

1) oat\generis\test\unit\extension\ComposerInfoTest::testExtractExtensionDependencies
Failed asserting that two arrays are equal.
--- Expected
+++ Actual
@@ @@
 Array (
-    'taoItemBank' => '*'
 )

/Users/mihaly.hadju/repos/sandbox/generis/test/unit/extension/ComposerInfoTest.php:49

ERRORS!
Tests: 308, Assertions: 960, Errors: 1, Failures: 1, Skipped: 20.

There's a comment on a - first glance - peculiar static call, which IMO requires further elaboration. Then from my side we are good to roll 🚀

common/ext/class.ExtensionsManager.php

common/oatbox/extension/ComposerInfo.php

gitromba

Right in the finish line. Passing unit tests 🚀 Comments resolved 🚀 Thanks for the fun @hutnikau

tikhanovichA

Review Checklist

New code is covered by tests (if applicable)
Tests are running successfully (old and new ones) on my local machine (if applicable)
New code is respecting code style rules
New code is respecting best practices
New code is not subject to concurrency issues (if applicable)
The feature is working correctly on my local machine (if applicable)
Acceptance criteria are respected
Pull request title and description are meaningful

…22/get_depencencies_from_composer

The changes were made in accordance with the comments.

Aleh Hutnikau added 2 commits April 6, 2020 17:24

get dependencies from composer

7955d9d

cache available packages

424af5f

hutnikau requested a review from jbout April 7, 2020 08:08

hutnikau mentioned this pull request Apr 7, 2020

use new Manifest class oat-sa/tao-core#2476

Merged

refactor manifest construct; fix manifest unit test

bc13f61

tikhanovichA reviewed Apr 8, 2020

View reviewed changes

common/ext/class.ExtensionsManager.php Outdated Show resolved Hide resolved

Aleh Hutnikau added 2 commits April 8, 2020 13:14

fix manifest unit test

fc2737e

add composer info test

ea5ede6

hutnikau requested review from bartlomiejmarszal, ktsirkou, ks16 and SergiiTao April 9, 2020 06:07

Aleh Hutnikau added 5 commits April 9, 2020 09:33

reduce complexity of getAvailablePackagesStatic

74554ae

reduce compexity of \oat\oatbox\extension\Manifest::getDependencies

59e7b4d

fix tao installation

95c46af

fix Manifest unit test

5958004

do not load extensons recursively

434a9df

Babacooll previously requested changes Apr 10, 2020

View reviewed changes

common/ext/class.ExtensionsManager.php Outdated Show resolved Hide resolved

common/oatbox/extension/ComposerInfo.php Outdated Show resolved Hide resolved

helpers/class.ExtensionHelper.php Show resolved Hide resolved

test/unit/ManifestTest.php Show resolved Hide resolved

Aleh Hutnikau added 4 commits April 10, 2020 10:11

use ComposerInfo constants

85903c0

retrieve root path if constant is not defined

72ae8a9

merge develop

c4899df

set version from composer after upate of extension

e4f3a41

ks16 previously requested changes Apr 20, 2020

View reviewed changes

Aleh Hutnikau added 3 commits November 18, 2020 12:06

merge develop

fff9217

fix composer.json file

fa282d2

Merge branch 'develop' of https://github.com/oat-sa/generis into TDR-…

60ebe32

…22/get_depencencies_from_composer

gitromba self-requested a review December 9, 2020 09:06

gitromba suggested changes Dec 11, 2020

View reviewed changes

Aleh Hutnikau added 2 commits January 12, 2021 12:17

Merge branch 'develop' of https://github.com/oat-sa/generis into TDR-…

403ff85

…22/get_depencencies_from_composer

switch to composer 2.0; fix ComposerInfo unit test

72b7e6a

Aleh Hutnikau added 2 commits January 12, 2021 15:12

remove getPackageInfo method

b1ba9f6

fix version retreival in manifest

9e658ad

Aleh Hutnikau added 4 commits January 12, 2021 18:20

fix checks extractor

85812fb

Merge branch 'develop' of https://github.com/oat-sa/generis into TDR-…

d3be5c1

…22/get_depencencies_from_composer

reduce complecity of getAvailablePackagesStatic

ccc60b5

remove unused methods from minifest class

5688cbc

Babacooll self-requested a review January 14, 2021 10:42

gitromba suggested changes Jan 14, 2021

View reviewed changes

Aleh Hutnikau added 3 commits January 15, 2021 13:19

Merge branch 'develop' of https://github.com/oat-sa/generis into TDR-…

ed705b9

…22/get_depencencies_from_composer

move extracting dependencies to ComposerInfo

8f3e7a8

update CompoerInfor unit test

957daff

gitromba suggested changes Jan 18, 2021

View reviewed changes

common/ext/class.ExtensionsManager.php Outdated Show resolved Hide resolved

common/oatbox/extension/ComposerInfo.php Show resolved Hide resolved

Aleh Hutnikau added 2 commits January 19, 2021 10:30

remove static call in extension manager

316d716

gix gitignore file

4ade9d4

gitromba approved these changes Jan 21, 2021

View reviewed changes

tikhanovichA self-requested a review February 3, 2021 10:19

tikhanovichA approved these changes Feb 3, 2021

View reviewed changes

Merge branch 'develop' of https://github.com/oat-sa/generis into TDR-…

4a37523

…22/get_depencencies_from_composer

feat!: get dependencies from composer file

2198028

hutnikau merged commit 046907e into develop Feb 12, 2021

hutnikau deleted the TDR-22/get_depencencies_from_composer branch February 12, 2021 13:23

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Tdr 22/get depencencies from composer #764

Tdr 22/get depencencies from composer #764

hutnikau commented Apr 7, 2020

ks16 left a comment

ks16 Apr 20, 2020

gitromba left a comment •

edited

Loading

hutnikau commented Jan 12, 2021

gitromba commented Jan 14, 2021

gitromba left a comment

gitromba commented Jan 14, 2021

gitromba left a comment •

edited

Loading

gitromba left a comment

tikhanovichA left a comment

Tdr 22/get depencencies from composer #764

Tdr 22/get depencencies from composer #764

Conversation

hutnikau commented Apr 7, 2020

ks16 left a comment

Choose a reason for hiding this comment

ks16 Apr 20, 2020

Choose a reason for hiding this comment

gitromba left a comment • edited Loading

Choose a reason for hiding this comment

hutnikau commented Jan 12, 2021

gitromba commented Jan 14, 2021

gitromba left a comment

Choose a reason for hiding this comment

gitromba commented Jan 14, 2021

gitromba left a comment • edited Loading

Choose a reason for hiding this comment

gitromba left a comment

Choose a reason for hiding this comment

tikhanovichA left a comment

Choose a reason for hiding this comment

gitromba left a comment •

edited

Loading

gitromba left a comment •

edited

Loading