-
Notifications
You must be signed in to change notification settings - Fork 49
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
[Suggestion] Extracting the JS business logic to reduce duplication #104
Comments
Moving JS code from PHP's HEREDocs, to corresponding Zombie process code which is written in JS is surely a good idea.
That is planned version, yes.
I with the only problems resulting in failed tests were on driver's side. In reality some of them are in Zombie itself or libraries it uses.
Since moving a code around is pure refactoring and not a feature introduction we don't really need to release 1.3 for it. The 1.2.1 will do just fine (since no public API or new functions are introduced). |
After working on #162, I thought about what doing this work would involved. Creating a full-featured node project for the driver would mean that we would have to deal with many files in our own code (probably one per driver method and a few more for the higher level codebase), and potentially some dependencies (at least a promise library as Node.js does not have native promise yet AFAIK and I would prefer using Promises than a callback hell). This would then make the driver harder to use:
This would also be a BC break for people using the driver, as this would be a rewrite from scratch (configuration of the Server would have to be different). So I thought about the ideal world:
So I suggest investigating this as a whole new driver rather than as a v2 of ZombieDriver. We could then decide whether we want to keep maintaining the existing ZombieDriver or deprecate it in favor of the RemoteDriver. What do you think about this idea @aik099 ? |
Interesting. This new NPM project would be NodeJS sever which internally runs Zombie and in fact can be used by anybody who needs it (not only Mink driver).
We can't guarantee, on PHP side, that a specific version of new NPM module is needed.
Agreed. |
I don't understand what you mean |
For example How do we ensure using Composer alone, that the MinkZombieServer running on X port on remote (or even same) server is running correct version supports API that we need? |
Currently, each method of the ZombieDriver needs to provide its JS business logic. This leads to code duplication. See for instance #103 which needs to be able to select a value for a radio button in 2 different methods.
My suggestion would be to move the business logic to a bunch of functions defined in the script running the zombie server itself. Then, the ZombieDriver would just have to use these functions when needed. This way, we could reuse some code by defining helper functions used by other functions, just like we do in MinkSelenium2Driver with a bunch of private methods in PHP for instance.
what do you think about this approach @aik099 ?
If we go this way, my suggestion would be to do the 1.2 release first (after fixing the tests), and then do the refactoring in a 1.3 release (to let us the time to ensure we don't introduce issues)
The text was updated successfully, but these errors were encountered: