EVerest / everest-demo

EVerest demo: Dockerized demo with software in the loop simulation
Apache License 2.0
14 stars 13 forks source link

Provide a demo of EVerest's existing automated testing capabilities #5

Closed drmrd closed 7 months ago

drmrd commented 8 months ago

This changeset includes a new demo that automatically runs the test in everest-core's startup_tests.py Pytest module, demonstrating how an aspect of EVerest's runtime behavior can be tested using EVerest's built-in Pytest tooling.

drmrd commented 8 months ago

The original intent for this work was to showcase an automated E2E test using EVerest's testing tools that actually runs through a charging session.

@couryrr-afs: I believe you created a new test that accomplishes this. Would it be feasible to contribute it as part of these changes?

couryrr-afs commented 8 months ago

@drmrd the code for that is in the everest-utils repo. I was running a container built locally based on the updated code base. I think we would need to go through some work to get that committed. As a proof of concept it was successful. We can work to get it all working together.

shankari commented 8 months ago

@couryrr-afs the existing automated tests (as such they are) are run on every PR, so I would expect them to work. The real goal of this task was to start experimenting with end-to-end, non-unit tests, preparatory to creating a full end-to-end test suite.

I don't see the challenge with committing the proof of concept. If you have it working locally, you can either include it in the current container (we can check out two repos) or create a new container for it, or if you really have to, docker commit a container that is manually set up the way you need it.

Is there an ETA for when the actual ask will be available?

drmrd commented 8 months ago

@shankari: As far as I'm aware, despite some of the step and image names in their GitHub Actions, EVerest's CI system only executes the startup test module that @couryrr-afs included in this PR:

@couryrr-afs: Were there any changes outside of everest-core that had to be made to fix the E2E tests? If not but you think your changes require more work before being merged into everest-core, you could still modify the manager Dockerfile to apply them. This can be done by, for instance, copying files into the image and moving them to the right directories before compiling the manager binary or by copying a patch file into the image that contains your changes and applying it to everest-core before compilation.

In terms of adding these changes back into EVerest, we can look at adding them to our backlog if they haven't been already.

couryrr-afs commented 8 months ago

@drmrd and @shankari There was no changes in functionality to the test themselves for this update. This is exposing the ability to have a docker container spin up and execute the existing pytests. In this case it is running a startup of everest and a startup and charging based on what was already there.

There were additional changes to run anther test that I created which had a failure case. It also included work to expose more of the messages in the tests which allows for more fine-grained assertions. If this is something I can commit I am happy to do so.

shankari commented 8 months ago

@couryrr-afs I believe these tests already run in a docker container as part of the everest-core CI https://github.com/EVerest/everest-core/blob/e52b21bd6ad4cdbfc98a132cae55032e1efca918/.ci/e2e/scripts/tests.sh#L4

As you can see from the related issue (https://github.com/US-JOET/everest-demo-private/issues/10), the goal here was not to simply run the existing tests in a container, but rather, to set up a test framework that can run the three demo scenarios that we currently have, but automated and without a UI.

This was intended to lay the framework for additional TDD going forward, and to have additional tests that we can add to CI/CD.

There were additional changes to run anther test that I created which had a failure case. It also included work to expose more of the messages in the tests which allows for more fine-grained assertions. If this is something I can commit I am happy to do so.

This seems more like the original goal. Can you clarify the reason why this might not be committed?

couryrr-afs commented 8 months ago

@shankari based on the issue I was given the goal was to re-use the demo services, docker compose, to automate the execution of the tests. This way they are not tied to the CI pipeline and anyone can run them by running the containers. The added functionality with messaging was in EVerest and the changes were not communicated to the community so we decided to hold off on pushing.

shankari commented 8 months ago

The added functionality with messaging was in EVerest and the changes were not communicated to the community so we decided to hold off on pushing.

I understand that the changes were not communicated to the community yet, but what better way to communicate than to 🎉 demo it working 🥳 ? We don't need to make changes to everest to set up a demo - just make a copy of the changed files in the demo repo and copy them in to the correct places while building the image - we do that all over the place in this repo.

Once we have coordinated with the community to get them into the upstream repo, we can remove the hack in here.

couryrr-afs commented 8 months ago

@shankari we do that for a lot of config files. This is related to source file for the code. If you are okay with that I can attempt to do so.

shankari commented 8 months ago

I am OK with this as a documented temporary hack, pending discussion and merge of source code. And I think we should use this approach in general to make progress on demos while: (1) waiting for the build process to be finalized, (ii) coordinating reviews with the broader community

couryrr-afs commented 8 months ago

@shankari I am attempting to get a new build for this out but am running into issues with the image. I like the hacky way for the time being.

drmrd commented 7 months ago

@shankari: This PR should be ready to merge after #4.

drmrd commented 7 months ago

@shankari: This PR is also ready for you to merge (and to review, if you haven't seen the E2E test that was added).

shankari commented 7 months ago

@drmrd I did review these two test cases last week. Happy to merge and make progress on this!

drmrd commented 7 months ago

@shankari Wonderful. Thanks much for all of the rapid-fire merging today! Very exciting to get these over the line.