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

Support for multiple codepools #82

Open
daveherbert opened this issue Sep 15, 2014 · 20 comments
Open

Support for multiple codepools #82

daveherbert opened this issue Sep 15, 2014 · 20 comments

Comments

@daveherbert
Copy link
Contributor

I've hit a bit of an issue in defining a target codepool for specs. I'm trying to write some specs for a module in the community codepool, in a project which has local specs.
In order to describe a spec for my community module, I've added the following to phpspec.yml:

mage_locator:
 [...snip...]
  code_pool: 'community'

This works fine for creating the particular spec, however I can no longer run against existing specs in the local codepool. It doesn't seem there is currently support for working with multiple codepools.

The issue now is how best to approach multiple codepool support. I'm thinking to add support to allow multiple values to be passed in the yml file, and then use a command option (i.e. "--codepool") to specify the target when describing.

It seems that might render the need for the value defined in the yml file redundant, as we could just run against all 3 codepools, and use the command option when describing. I'm not sure I'm keen on that idea though.

Any thoughts? 😄

@shanethehat
Copy link
Member

I'm wondering if the value in the yaml file should reflect the describe target, and when using the run command the resource manager should search all code pools for specs to run if no specific query is given.

@shanethehat
Copy link
Member

Including @amacgregor as the author of the current implementation

@daveherbert
Copy link
Contributor Author

I still favour being able to have an override for describing, but maybe the yaml value reflects the default describe target?

@shanethehat
Copy link
Member

Some time ago there was an argument to the command that set a code pool, although I don't believe it worked. It would be nice to have that back now that Allan has done all the groundwork to support it.

@debo
Copy link
Contributor

debo commented Sep 15, 2014

My advice on this, since we have the command to specify the code pool during describe, is to then have the locator to behave like Magento in terms of folder priority local -> community -> core.

@shanethehat
Copy link
Member

I don't think we do have the command to specify during describe any more, although I think it should be reinstated to override the yaml configuration.

Would there be any benefit to having the run locators care about the code pool order? If a user calls run without specifying a path, I would expect the tool to run specs from any code pool it finds them. Or do you just mean that it should search in that order but still run all the specs it finds?

@alistairstead
Copy link
Contributor

I like that solution Shane.

Sent from Mailbox

On Mon, Sep 15, 2014 at 1:18 PM, Shane Auckland [email protected]
wrote:

Including @amacgregor as the author of the current implementation

Reply to this email directly or view it on GitHub:
#82 (comment)

@debo
Copy link
Contributor

debo commented Sep 15, 2014

@shanethehat either ways

@jamescowie
Copy link
Member

I think we spoke about the optional command line argument but at the time we opted for yaml based configuration option. What I see as being a working solution could be:

phpspec.yml still contains the default code pool for running and describing objects ( Useful if only developing a single module or for a single code pool )

However the command line could have a new option for describing and running specs:

phpspec run --codepool=local/community
phpspec descibe:block magespec_test/blockfile --codepool=local/community

If the command line argument is set then it takes priority over configuration.

@alistairstead
Copy link
Contributor

How about codepool in the yml being an array of pools?

Then have the flag to inform run and describe commands. If there is an array in the yml and the flag is not specified then an interactive prompt provides a selection:

Which code pool would you like to use?

1. community
2. core
3. local
4. all

You could add another config value to define 4 as the default for run?

@daveherbert
Copy link
Contributor Author

@jamescowie That sounds pretty much how I envisaged it to work for the command line option.

Would we then treat the yml value as a default option for using describe for new classes, but run against all codepools? Alternatively I was thinking we could define an array of codepools to run against in the yml, perhaps with the first one being the default for describe?

@jamescowie
Copy link
Member

I like @alistairstead approach so phpspec.yml could look like:

extensions: [MageTest\PhpSpec\MagentoExtension\Extension]
mage_locator:
  spec_prefix: 'spec'
  src_path: 'public/app/code'
  spec_path: 'spec/public/app/code'
  code_pool: ['community','local']
  default_run_code_pool: 'all'
  default_describe_code_pool: 'local'

This would allow us to define a default option via configuration as a fall back, When running PHPSpec to describe or run without a command line flag we would show the interactive prompt for where to run specs from or where to describe them. We would then still offer a CLI overwrite to bypass the prompt screen with validation to ensure that the config is correct. E.g. describe core without core being configured to run..

I think this is the complete solution but it might be worth exploring the CLI only option with yaml configuration being the default fallback as a first pass.

The most basic solution is to have 2 config files for different code pools and pass the config file when running PHPSpec via -c flag. Far from ideal :)

@shanethehat
Copy link
Member

I'm not sure that any code pool needs to be set for running specs.

The standard behaviour of phpspec is that when the run command is not
provided with a target path it will search for all supported spec classes
in the specs directory. I think that if a project contains specs from
multiple code pools, the default behaviour of the run command should be to
run all of them.

Too only run the specs from a single code pool the use would pass a path to
the run command:

phpspec run spec/public/app/code/community

I also don't think we should make it possible to use the core code pool, as
that would suggest that changing core code is somehow okay. Community and
local should be the only legal values.

@debo
Copy link
Contributor

debo commented Sep 15, 2014

@shanethehat I was thinking about the same. I think the code pool should go away from yaml configuration and in good phpspec style the tool should guide the user through the auto generation process. So when describing the the class we are speccing MageSpec should ask us if we want create the spec first and for which code pool afterwards or viceversa, whichever fits better. When we then run the specs, phpspec standard behaviour should be preserved and anything that is under spec should be run unless otherwise specified.

@shanethehat
Copy link
Member

I'm not so fond of introducing another manual step into the description process. I think that for the majority of users, one code pool per project will be sufficient, and so being able to define that somewhere once makes a lot of sense to me. Maybe instead of defaulting to local like we do now, we could prompt the user if no value is set in the yaml file.

@debo
Copy link
Contributor

debo commented Sep 15, 2014

What about make the code pool part of the model description then maybe? I
understand your concerns and I can identify with them, that is why original
we wanted to be very strict, unfortunately many people seemed to miss the
possibility to use both code pool in a flexible way. However I think that
hit enter once more is not that much hassle I think, we can also store the
last choice and offer it as default at the next run. Think about how the
command "hobo config" works, I has something like that in mind.

On 15 September 2014 23:16, Shane Auckland [email protected] wrote:

I'm not so fond of introducing another manual step into the description
process. I think that for the majority of users, one code pool per project
will be sufficient, and so being able to define that somewhere once makes a
lot of sense to me. Maybe instead of defaulting to local like we do now, we
could prompt the user if no value is set in the yaml file.


Reply to this email directly or view it on GitHub
#82 (comment).

@shanethehat
Copy link
Member

Unfortunately I've never used Hobo :(

These scenarios are how I would imagine the existing functionality could be extended to allow a project with specs in both code pools to be used effectively. Please chop and change them!

Scenario: User has not defined a code pool
Given that there is no code pool defined in config
When the user describes a Magento object
Then the user will be prompted to select a target code pool
And the spec will be created in the selected code pool

Scenario: User's code pool choice is remembered
Given that there is no code pool defined in config
And the user describes a Magento object
And selects "local" as the target code pool
When the user describes a second Magento object
Then the user will be offered "local" as the default choice of code pool

Scenario: User has defined a code pool in config
Given that "local" is configured as the default code pool
When the user describes a Magento object
Then the spec will the created in the "local" pool

Scenario: User overrides the default code pool
Given that "local" is configured as the default code pool
When the user describes a Magento object
And sets the codepool flag to "community"
Then the spec will be created in the "community" code pool

Scenario: User may not target the core code pool from config
Given that "core" is configured as the default code pool
When the user describes a Magento object
Then they will see an illegal code pool error

Scenario: User may not target the core code pool from override
Given that "local" is configured as the default code pool
When the user describes a Magento object
And sets the codepool flag to "core"
Then they will see an illegal code pool error

Scenario: User can run specs from all code pools
Given that there is a "local" code pool spec
And there is a "community" code pool spec
When the user calls the run command with no path
Then all the specs will be executed

Scenario: User can run specs from a single code pool
Given that there is a "local" code pool spec
And there is a "community" code pool spec
When the user calls the run command with the community code pool path
Then only the community spec will be executed

@debo
Copy link
Contributor

debo commented Sep 16, 2014

Scenario: User may not target the core code pool from config
Given that "core" is configured as the default code pool
When the user describes a Magento object
Then they will see an illegal code pool error
And a S.W.A.T. squad is sent to took user down

;)

@shanethehat
Copy link
Member

@daveherbert did you make any progress with implementing this issue?

@daveherbert
Copy link
Contributor Author

@shanethehat I made a good start on it and I've gone back to it a few times since, haven't touched it in a couple of months though. I should probably do something about that.

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

No branches or pull requests

6 participants