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

[POC - Behat] Add RepositoryContext trait #1584

Merged
merged 3 commits into from
May 3, 2016

Conversation

miguelcleverti
Copy link

As decided BehatBundle is to be deprecated and separated between the kernel and platformUI bundle. The idea is to make the Behat easier to maintain, having less repo dependencies should help releases and versions also.

This PR moves the RepositoryContext trait into the kernel. This is trait contains all the methods related to the repo . Since this is to be used by any contexts that need the repo it should be in the kernel to be easily included and not duplicate.

Related PRs:

@miguelcleverti
Copy link
Author

ping @bdunogier @andrerom @joaoinacio

private $adminUserId = 14;

/**
* @var Repository
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please use the full namespace in phpdoc (same below).

@miguelcleverti miguelcleverti force-pushed the BehatRepositoryContext branch from cb0393a to a973922 Compare March 8, 2016 10:27
@miguelcleverti
Copy link
Author

@bdunogier Improved the docs in the Context and added a more detailed to a dedicated behat docs file.

@bdunogier
Copy link
Member

How is the repository injected in the trait again ?

@miguelcleverti
Copy link
Author

@bdunogier the repository has to be injected into the context in which this trait is used, using the ArgumentResolver (#1586), pass it through the behat.yml into the constructor or any other way...

@miguelcleverti miguelcleverti force-pushed the BehatRepositoryContext branch from 5727a34 to 6e624e5 Compare April 26, 2016 15:33
/**
* @param eZ\Publish\API\Repository\Repository $repository
*/
public function setRepository(Repository $repository)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be protected, not public. Each context that uses the trait would expose setRepository & loginAdmin, while it is not necessary.

@bdunogier
Copy link
Member

bdunogier commented Apr 29, 2016

I'm not sure what to say. It is fine, and it works, but for many little reasons it does not sound right...

But let's say that if you:

  • fix the visibility
  • add a getter for $repository and make the property private

I'll be +1 then.

@miguelcleverti
Copy link
Author

@dpobel changed the visibility and added the getter

@miguelcleverti miguelcleverti force-pushed the BehatRepositoryContext branch 2 times, most recently from de9242c to 09ade8b Compare May 2, 2016 10:53
@miguelcleverti miguelcleverti force-pushed the BehatRepositoryContext branch from 09ade8b to 38bb920 Compare May 2, 2016 11:10
@miguelcleverti
Copy link
Author

miguelcleverti commented May 2, 2016

Actually the loginAdmin() must be public or else behat can't use it for the BeforeScenario Hook

@miguelcleverti
Copy link
Author

ping @bdunogier

@bdunogier bdunogier merged commit 3dc6777 into ezsystems:master May 3, 2016
@bdunogier
Copy link
Member

And it is merged. Let's see during the 16.06 cycle how it behaves. I'll use it in a scenario of mine right away.

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

Successfully merging this pull request may close these issues.

2 participants