medic / cht-conf

A command-line interface for configuring Community Health Toolkit applications
https://communityhealthtoolkit.org
GNU Affero General Public License v3.0
23 stars 25 forks source link

feat(#425): Create e2e test setup #610

Closed sugat009 closed 2 months ago

sugat009 commented 7 months ago

Description

Add a shell script that downloads the cht-core docker-helper file and sets cht-core up for e2e testing. The shell script also destroys cht-core containers using the same docker-helper after testing has been completed.

medic/cht-conf#[425]

Code review items

License

The software is provided under AGPL-3.0. Contributions to this project are accepted under the same license.

sugat009 commented 7 months ago

Is there a reason you think it might be better to use a shell script for the orchestration of the tests and to trigger the mocha run vs using mocha to orchestrate the setup and teardown of everything by running shell commands...

That's a very intriguing question @jkuester . I don't have a strong preference, but upon reflection, even though we utilize Mocha for setup and teardown orchestration, we essentially end up executing shell commands, albeit through JavaScript. In terms of choosing between the two, I'd say each approach has its own advantages and disadvantages. However, if I were to lean towards one, I'd opt for writing it in shell scripts. They are simpler to write, have lower resource overhead, and there's the rare scenario where a task is feasible in the shell but not as straightforward in a programming language like JS, TS, or Python.

My main motivation for asking is that in cht-core, we settled on orchestrating everything (e.g. the integration tests) through mocha so that launching the tests via a mocha command will do all the setup/teardown of docker, etc.

My apologies for not being aware of this. I must admit, I'm not a shell expert myself 😅. However, if the technical preference leans towards using JavaScript for these types of tasks, I have no objections and I'm willing to rewrite this in JS.

Would be nice to just have a one-and-done command to run instead of the script having to be "interactive"....

Yes, this would be a nice feature to have. I'm thinking of adding a flag to docker-helper such that it does not prompt any input and issues everything in default in the future.

jkuester commented 7 months ago

@m5r @tatilepizs what are your thoughts on orchestrating the tests with a shell script vs Mocha? I agree with Sugat that a shell script will be more straightforward, but at the same time I put a lot of value in consistency and so it would be nice to match cht-core. @tatilepizs do you have any context for why we went with mocha for the cht-core e2e/integration tests?

m5r commented 7 months ago

I also like the simplicity of bash and would usually prefer it over javascript for tiny, well-defined scripts. In this case I would lean towards javascript for two main reasons:

tatilepizs commented 7 months ago

I agree with @jkuester and @m5r about consistency, using different things could be complicated especially when we are learning about the project. I understand @sugat009's point about simplicity, but I really like the organization and structure of having just one way of doing things.

do you have any context for why we went with mocha for the cht-core e2e/integration tests?

No I don't, when I joined Medic that decision was already made 😅

However, I think that in one of the projects from ecosystem, they use something different than mocha to run tests. Am I right @lorerod ?

lorerod commented 7 months ago

Hi @m5r @jkuester @tatilepizs @sugat009, we experienced with Jest in the cht-interoperability repo, and it was mainly for unit and API tests. Our e2e test doesn't require web browser testing as the repo publishes an API, not a web interface. For API e2e testing, we use Jest with Supertest. @medic/ecosystem's main arguments for choosing Jest in the cht-interoperability repository instead of the already-used Mocha were that Jest does not need other libraries to work and is focused on simplicity. We didn't want to overcomplicate a simple project with many extra dependencies. Our experience using jest was great.

I understand the benefits of having only one way of doing things. Cht-conf is more closely related to cht-core. Does cht-conf have unit tests implemented? What was used for that? We could choose the same tool, which may also suit our needs for the e2e test.

jkuester commented 7 months ago

That is helpful context @lorerod and thank you everyone for the input! cht-conf is currently using mocha for running the unit/integration tests. Based on the above feedback, I think it makes sense to go with a similar approach as in the cht-core integration/e2e tests where we use mocha as the entry-point/orchestrater of the test run. @sugat009 let me know if more clarity is needed here or if you run into something that makes this seem like a bad idea!

sugat009 commented 7 months ago

Thanks, everyone for your feedback and input, it makes complete sense to go with mocha for orchestration for consistency. I'll revise my changes to match it with how we do it in cht-core using mocha.

garethbowen commented 3 months ago

@sugat009 Just checking up with where we're at here?

sugat009 commented 3 months ago

@garethbowen This task has been on pause for a while now as I've been assigned other tasks such as https://github.com/medic/cht-android/issues/308 and https://github.com/moh-kenya/config-echis-2.0/issues/1871. The progress before transitioning to pause is that I've re-written this in JS up until the point where the docker helper file is run and I was working on providing it input like in line 30 of test/e2e/test_runner.sh of the current PR state.

m5r commented 2 months ago

Closing in favor of medic/cht-conf#620. Reason is Josh is out and there is no way to merge this to 425-improve-testing-on-cht-conf without him or an admin removing his request for changes. The setup is done and orchestrated with mocha, I'm adding a test before going through a review.