Added new tool - expressive-module (#8 and #9) #12

michalbundyra · 2017-02-17T16:14:17Z

New tool vendor/bin/expressive-module

Tool provide following commands:

create - create skeleton of the expressive module and register it
register - register module in configuration and composer autoloading
deregister - deregister module from configuration and composer autoloading

Tool accept following arguments:

--composer|-c path to composer binary
--modules-path|-p - path to modules directory

TODO:

tests
- actions
  - create
  - register
  - deregister
- main command
- help
implement disable module's composer autoloading - will be nice to have it in zfcampus/zf-composer-autoloading as there is also registering module in composer.json - done in [v2.0] [BC Break] Rewrite library with ConsoleHelper + tests zfcampus/zf-composer-autoloading#8
would be nice to refactor zendframework/zend-component-installer then we can have here less dependencies - now because we are using this library we require here composer/composer - done in [BC Break] Refactor and injector interface changes zend-component-installer#34.

Resolves #8 and #9

weierophinney · 2017-02-17T17:43:52Z

Syntax is perfect - exactly what I'd expect! Looking forward to reviewing the final iterations!

michalbundyra · 2017-02-22T17:32:32Z

@weierophinney It's ready for review, tests are failing but some of them are fixed in #10, so it should be merged first. Of course it needs also zend-component-install 0.7.0 release and update requirements in composer.json.
What is missing here - messages on success. Now we don't have any messages, so it's confusing if it was done successfully or not. Just response code is 0 on success. I need to think about it a bit more how to do it nicely, or maybe you have an idea?

weierophinney · 2017-02-22T17:59:00Z

@webimpress I've tagged zend-component-installer 0.7.0 at this time, and merged #10.

As for messages, how about:

"Created module <module name> in <path>, and generated autoloading rules"
"Registered autoloading rules and added configuration entry for module <module>"
"Removed autoloading rules and configuration entries for module <module>"

Tool provide following commands: - "create" - create skeleton of the expressive module and register it - "register" - register module in configuration and composer autoloading - "deregister" - deregister module from configuration and composer autoloading Tool accept following arguments: - "--composer|-c" path to composer binary - "--modules-path|-p" - path to modules directory

Eliminated dependency of composer/composer package. Now to inject/remove methods we don't need to pass IOInterface.

…mands

Redirect stderr to stdout 2>&1 Works also on Windows.

Added catching RuntimeException from zf-composer-autoloading and throwing module RuntimeException. Now we have then consistent interface, and process method throws only exception from the module. Updated PHPDocs to reflect these changes.

Now commands from zfcampus/zf-composer-autoloading are invoked via "process" method instead of "__invoke". Also disabled moving module class for the module on enabling autoloading.

I've notice issue with constant and Mockery. If we use Mockery to mock class, than we can't access to the constants. This change uses constants from another class, so we have access on mocking ConfigAggregatorInjector.

- zendframework/zend-component-installer ^0.7 - zf-composer-autoloading ^2.0

michalbundyra · 2017-02-22T22:32:45Z

@weierophinney Rebased, used latest stable dependencies and added success messages as suggested. Ready for review.

weierophinney

Looks fantastic, @webimpress ! There are a few minor changes I've suggested, and one larger one that, honestly, I can go either way on (whether or not to memorize $modulePath). Let me know how you want to proceed on that, and I can merge.

weierophinney · 2017-03-01T14:53:20Z

composer.json

@@ -40,7 +44,8 @@
    "bin": [
        "bin/expressive-migrate-original-messages",
        "bin/expressive-pipeline-from-config",
-        "bin/expressive-scan-for-error-middleware"
+        "bin/expressive-scan-for-error-middleware",
+        "bin/expressive-module"


These should be alphabetized.

weierophinney · 2017-03-01T14:56:16Z

src/Module/Command/Create.php

+/**
+ * The configuration provider for the %1$s module
+ *
+ * @see https://docs.zendframework.com/zend-component-installer/


Nice touch! Linking to the documentation will definitely assist new developers.

weierophinney · 2017-03-01T14:58:40Z

src/Module/Command/Create.php

+     */
+    private function createDirectoryStructure($moduleName)
+    {
+        if (file_exists($this->modulePath)) {


Considering that $modulePath is calculated by process(), perhaps it should be passed as an argument to each of this method and createConfigProvider(), instead of memoized as a property? That would be one less state for the instance to track.

Changed - property removed, now method has two parameters.

weierophinney · 2017-03-01T15:00:03Z

src/Module/Help.php

+<info>Commands:</info>
+
+  <info>help</info>          Display this help/usage message
+  <info>create</info>        Create skeleton of the expressive module and register it


s/skeleton of/source tree for/

weierophinney · 2017-03-01T15:01:16Z

src/Module/Help.php

+
+  <info>help</info>          Display this help/usage message
+  <info>create</info>        Create skeleton of the expressive module and register it
+  <info>register</info>      Register module in configuration and composer autoloading


"Register module in application configuration, and enable autoloading of module via composer."

weierophinney · 2017-03-01T15:01:39Z

src/Module/Help.php

+  <info>help</info>          Display this help/usage message
+  <info>create</info>        Create skeleton of the expressive module and register it
+  <info>register</info>      Register module in configuration and composer autoloading
+  <info>deregister</info>    Deregister module from configuration and composer autoloading


"Deregister module from application configuration, and disable autoloading of module via composer."

weierophinney · 2017-03-01T15:03:26Z

test/Module/Command/CreateTest.php

+    {
+        $baseModulePath = sprintf('%s/my-modules/MyApp', $this->dir->url());
+
+        $mkdir = $this->getFunctionMock('Zend\Expressive\Tooling\Module\Command', 'mkdir');


I was not aware of getFunctionMock()! Awesome!

Please not this is not from PHPUnit but from php-mock library. Integration with PHPUnit is via php-mock/php-mock-phpunit.

Ah, okay - did you add that dependency, or does PHPUnit depend on it itself? Either way, it's a nice library to know about.

Yes, I added:

"php-mock/php-mock-phpunit": "^2.0 || ^1.1.2",

There is also integration library with prophecy and mocker.
https://github.com/php-mock
I've chosen integration with PHPUnit as this is the main testing library. I know that many mocking we are doing recently using prophecy, so maybe better choice will be php-mock-prophecy?
Here is the library with the main functionality:
https://github.com/php-mock/php-mock

weierophinney · 2017-03-01T15:05:24Z

test/Module/Command/DeregisterTest.php

+namespace ZendTest\Expressive\Tooling\Module\Command;
+
+use Mockery;
+use Mockery\Adapter\Phpunit\MockeryPHPUnitIntegration;


Is this what allows you to not need to specify @runInSeparateProcess for tests that overload classes via Mockery?

Hm... this is recommended in Mockery documentation:
http://docs.mockery.io/en/latest/reference/phpunit_integration.html

I was trying to remove @runInSeparateProcess but some tests on some versions (php versions/dependencies) were failing. As you can note in some tests we still have this annotation.

I've somehow missed that documentation page previously, so, again, a nice thing to know about!

@weierophinney

- per @weierophinney

@weierophinney

- per @weierophinney

@weierophinney

- per @weierophinney

weierophinney

Changes look perfect! Thanks, @webimpress !

Added new tool - expressive-module (#8 and #9)

Close #12 Fixes #8 Fixes #9

This was referenced Feb 18, 2017

[BC Break] Refactor and injector interface changes zendframework/zend-component-installer#34

Merged

[v2.0] [BC Break] Rewrite library with ConsoleHelper + tests zfcampus/zf-composer-autoloading#8

Merged

michalbundyra changed the title ~~[WIP] Added new tool - expressive-module (#8 and #9)~~ Added new tool - expressive-module (#8 and #9) Feb 22, 2017

michalbundyra added 24 commits February 22, 2017 20:45

Used refactor version of zend-component-installer

39f522f

Eliminated dependency of composer/composer package. Now to inject/remove methods we don't need to pass IOInterface.

PHPDocs fixed allowed param type

fc0e592

Removed unused import

714db33

Added command chains implementation

ef46142

Removed redundant blank lines

6ccf696

Updated chmod for bin file (+x)

9a46e05

Suppressing output on checking composer binary

375a205

Updated to zf-composer-autoloading v2-dev with enable and disable com…

d91f8d0

…mands

Method getCommand expects only first script argument

3f63e3f

Suppress errors on checking composer binary

2b3222f

Redirect stderr to stdout 2>&1 Works also on Windows.

Updated PHPDocs comment

fbd63bf

Allow to use with zend-code 2.6.3

b4e0315

Added expressive-module Help tests

e4ad920

Updated to latest dev version of zfcampus/zf-composer-autoloading

eed75d2

Now commands from zfcampus/zf-composer-autoloading are invoked via "process" method instead of "__invoke". Also disabled moving module class for the module on enabling autoloading.

Construct command without moduleName and expect module name in process

acd5d05

Removed "chmod" property in Create command

bdc4c4c

Use constant from interface, not from injector

7813e67

I've notice issue with constant and Mockery. If we use Mockery to mock class, than we can't access to the constants. This change uses constants from another class, so we have access on mocking ConfigAggregatorInjector.

Added tests for commands "create", "register", "deregister"

f388976

Changed $projectDir property visibility to private

004d4a1

Updated arguments checks and messages

fa67770

Added tests for Module\Command

62366ce

Updated to released dependencies

ffdcd0c

- zendframework/zend-component-installer ^0.7 - zf-composer-autoloading ^2.0

michalbundyra added 2 commits February 22, 2017 21:01

Updated travis configuration to work with legacy deps

db3d178

Added success messages

1b4763f

weierophinney suggested changes Mar 1, 2017

View reviewed changes

michalbundyra added 3 commits March 1, 2017 15:29

Reorder bin scripts in composer - alphabetica order

29a676c

- per @weierophinney

Removed class property and pass modulePath as argument

1d35ea7

- per @weierophinney

Updated help messages

b3952af

- per @weierophinney

weierophinney approved these changes Mar 1, 2017

View reviewed changes

weierophinney added this to the 0.3.0 milestone Mar 1, 2017

weierophinney added the enhancement label Mar 1, 2017

weierophinney self-assigned this Mar 1, 2017

weierophinney merged commit b3952af into zendframework:master Mar 1, 2017

weierophinney added a commit that referenced this pull request Mar 1, 2017

Merge pull request #12 from webimpress/feature/module-tools

419d679

Added new tool - expressive-module (#8 and #9)

weierophinney added a commit that referenced this pull request Mar 1, 2017

Added CHANGELOG for #12

fdf06aa

weierophinney added a commit that referenced this pull request Mar 1, 2017

Merge branch 'hotfix/12'

f78908f

Close #12 Fixes #8 Fixes #9

michalbundyra deleted the feature/module-tools branch March 1, 2017 16:53

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Added new tool - expressive-module (#8 and #9) #12

Added new tool - expressive-module (#8 and #9) #12

michalbundyra commented Feb 17, 2017 •

edited

Loading

weierophinney commented Feb 17, 2017

michalbundyra commented Feb 22, 2017

weierophinney commented Feb 22, 2017

michalbundyra commented Feb 22, 2017

weierophinney left a comment

weierophinney Mar 1, 2017

weierophinney Mar 1, 2017

weierophinney Mar 1, 2017

michalbundyra Mar 1, 2017

weierophinney Mar 1, 2017

weierophinney Mar 1, 2017

weierophinney Mar 1, 2017

weierophinney Mar 1, 2017

michalbundyra Mar 1, 2017

weierophinney Mar 1, 2017

michalbundyra Mar 1, 2017

weierophinney Mar 1, 2017

michalbundyra Mar 1, 2017

weierophinney Mar 1, 2017

weierophinney left a comment

Added new tool - expressive-module (#8 and #9) #12

Added new tool - expressive-module (#8 and #9) #12

Conversation

michalbundyra commented Feb 17, 2017 • edited Loading

weierophinney commented Feb 17, 2017

michalbundyra commented Feb 22, 2017

weierophinney commented Feb 22, 2017

michalbundyra commented Feb 22, 2017

weierophinney left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

weierophinney left a comment

Choose a reason for hiding this comment

michalbundyra commented Feb 17, 2017 •

edited

Loading