PlatformUI Context #230
PlatformUI Context #230
Conversation
|
|
||
| class PlatformUI extends Context | ||
| { | ||
| const USER = "admin"; |
There was a problem hiding this comment.
We should make that (+ password) configurable somehow. It is quite convenient to run BDD tests on an existing instance without changing your login & password.
|
I'm a bit lost here: are the FieldType scenarii the same than then ones from #211 ? I can't see common commits between both pull requests. |
|
@miguelcleverti / @pcardiga why is there a integer.feature here and also another one in #234 ? |
|
@bdunogier / @andrerom I added the scenario by mistake in this branch, I removed it. |
It makes sense, but wouldn't it be more consistent if the default login/password was in behat.yml.dist ? That way people know immediately how to change it. |
|
@bdunogier: do you mean |
|
Seems @lolautruche changed CS for all repos no? or was it temp problem maybe? :) |
|
@andrerom only for PlatformUIBundle. But yes, I changed the script on Jenkins |
|
@lolautruche @andrerom some of the files are not related to my PR, should I change only my files or do you want me to fix everything? |
|
@miguelcleverti Don't change anything for now. |
|
@miguelcleverti #239 is now merged. Please rebase your branch now and update your PR to comply PSR-2. Thanks :-) |
miguelcleverti
force-pushed
the
PlatFormUIBDDCore
branch
2 times, most recently
from
dbe6b55
to
9259b24
Compare
|
Changed the cs and squashed some commits |
|
looks better now, your pov @bdunogier ? |
|
|
||
| /** | ||
| * File containing the main context class for PlatformUI | ||
| * This flie contains the mapping of the BDD sentences to the functions that implement them |
|
The scenario are very confusing because you call almost everything side menu, navigation menu or worse link. Widgets have names, it would be nice to try to stick to them, here's schema to help:
|
| @@ -0,0 +1,132 @@ | |||
| Feature: Login on PlatformUI | |||
There was a problem hiding this comment.
Is it really what is tested here ? Besides the first scenario, they look more like navigation scenarios.
|
Ping @miguelcleverti. |
miguelcleverti
force-pushed
the
PlatFormUIBDDCore
branch
from
9259b24
to
3b96d0e
Compare
|
@bdunogier, @dpobel, @andrerom the changes to the domain language that @dpobel referred were mostly already done, the standard feature file was outdated. I improved the bar menus button clicking, now there is a difference between the discovery, the action and the edit menu bar. I also rebased to master. |
|
|
||
| /** | ||
| * @Given I click (on) the edit action bar button :button | ||
| * Click on a PlatformUI action bar |
There was a problem hiding this comment.
Nitpick: should probably specify the "Edit" action bar here?
miguelcleverti
force-pushed
the
PlatFormUIBDDCore
branch
from
110781f
to
e70f2ef
Compare
| * | ||
| * @param string $button Text of the element to click | ||
| */ | ||
| public function overNavigationZone($zone) |
|
During the workshops, it was asked if there was like a minimal behat test running on PlatformUI. The answer was unfortunately no. Seeing this PR, it seems to me that we have like everything we need or close. If we extract the Authentication Context + the Common one, we can have a simple scenario that tries to login on PlatformUI, and asserts that the homepage is how it should be. This would ensure that PlatformUI works (e.g. you can login, and the basic content is retrieved) on pull-requests. It's a small step, bug a big insurance. Could we do that ? Ping @andrerom @miguelcleverti @dpobel. |
|
Alternative is to merge it, since it passes, and update the sentences as we use them. I'm not a huge fan* of committing code we don't use and agreed upon, but I could agree. |
|
+1 |
1 similar comment
|
+1 |
|
Merged, please rebase issues that depend on this. |
Reworked PlatformUI BDD implementation
Removed JS completely and used the proper Mink methods to do the same thing. FieldTypes sentences (as seen in #211) base implementations are also done in conjunction with ezsystems/BehatBundle#24