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

Possible improvement #10

Open
rmNyro opened this issue Aug 28, 2017 · 2 comments
Open

Possible improvement #10

rmNyro opened this issue Aug 28, 2017 · 2 comments

Comments

@rmNyro
Copy link
Contributor

rmNyro commented Aug 28, 2017

Hi @mycargus,

As mentioned earlier I will be using a slightly different version of what you built. I'll be using it but from outside and I think I'll use it on other projects.
From there, I have seen 2 things I want to discuss:

  • dignhy/dory mention suppression (except from the last part and keep compatibility
  • outsourcing tests/ folder as you never separate code from tests, it doesn't feel right to me

The first observation is based on 1816244 since it gets rid of previous logic relying on dinghy/dory which wasn't working on linux anyway, even with dory installed.
So I'm thinking, if you accept the PR maybe it's time to rewrite the README and keep mention of those tools only in the last right before "## I want to see the app under test. How can I do that?" since only paragraphs from this one make use of the d/d.

For the second, I'd say a rework with an inverted logic may be great.
My vision is that anyone willing to use the grid has a project with tests or is used to have tests in the folder of a project and would use the grid as an external source. However the grid is currently in need of a simple docker image (which moreover isn't necessarily the case) and the tests/ folder within the grid's folder. Maybe via npm with docker as only dependency, something like npm i -g nightwatch-docker-grid-cli && nightwatch-grid --files=./tests --debug=true launch with some (if not all) optional parameters.

If only the grid was simple enough to be built using something like docker run. Unfortunately I haven't found the way to get ride of the docker cpying at the end of the tests script (from your last commits) to copy debug output.

Tell me what you think.

@mycargus
Copy link
Owner

Awesome work on 0dfe73a ! I like the changes.

I completely agree with the need you identified. It's something I considered when I first built the project as a proof of concept. At the time I chose to focus on a working dockerized grid instead. I would love to see this project evolve into something that can be pulled into existing projects without the project owners having to move their test suite.

@rmNyro
Copy link
Contributor Author

rmNyro commented Aug 29, 2017

Thank you. I'm sure it can be improved.
I will need to focus on my frontend work for a while as I didn't really find the way to make it usable as an external component. Here are the problems I encountered and for which I didn't find a way to fix:

  1. removing the need of the scripts in bin/ (mostly because of the docker cp ...)
  2. externalizing the SUT's image (put it out of the compose file)

This project will evolve in time, I'm not worried. :)

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

No branches or pull requests

2 participants