-
Notifications
You must be signed in to change notification settings - Fork 14
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
Provide a demo of EVerest's existing automated testing capabilities #5
Conversation
9dcacf4
to
0e8ab1a
Compare
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? |
@drmrd the code for that is in the |
@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, Is there an ETA for when the actual ask will be available? |
@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 In terms of adding these changes back into EVerest, we can look at adding them to our backlog if they haven't been already. |
@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. |
@couryrr-afs I believe these tests already run in a docker container as part of the 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.
This seems more like the original goal. Can you clarify the reason why this might not be committed? |
@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. |
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. |
@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. |
I am OK with this as a documented temporary hack, pending discussion and merge of source code. |
@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. |
993abfb
to
f52f1b6
Compare
7ce57e8
to
6b09d38
Compare
Signed-off-by: Daniel Moore <9156191+drmrd@users.noreply.github.com>
Signed-off-by: Coury Richards <146002925+couryrr-afs@users.noreply.github.com> Signed-off-by: Daniel Moore <9156191+drmrd@users.noreply.github.com>
Signed-off-by: Daniel Moore <9156191+drmrd@users.noreply.github.com>
Signed-off-by: Daniel Moore <9156191+drmrd@users.noreply.github.com>
6542f8e
to
954b5d2
Compare
@shankari: This PR is also ready for you to merge (and to review, if you haven't seen the E2E test that was added). |
@drmrd I did review these two test cases last week. Happy to merge and make progress on this! |
@shankari Wonderful. Thanks much for all of the rapid-fire merging today! Very exciting to get these over the line. |
Removed part of patch + Added better setChargingProfile script
This changeset includes a new demo that automatically runs the test in
everest-core
'sstartup_tests.py
Pytest module, demonstrating how an aspect of EVerest's runtime behavior can be tested using EVerest's built-in Pytest tooling.