initial checkin of vk tests for dinkum. still needs integration into … #3
Conversation
There was a problem hiding this comment.
Hey this is looking good. I like the slimmed down run time so we aren't running things that aren't actually getting tested. Over time we can re-add services as our integration tests mover closer to become true "end-to-end".
A few notes and questions for consideration below.
docker/packages-run.txt
Outdated
| plasma-nano | ||
| plasma-nm | ||
| plymouth | ||
| plymouth-themes | ||
| python3-pip | ||
| python3-rpi.gpio |
There was a problem hiding this comment.
Are these no longer necessary for the Mark II? Or just for the VK tests?
Great if we can remove them, otherwise a dedicated package list might be necessary as above.
There was a problem hiding this comment.
This is actually a bug. Nothing is to be removed, just 2 lines added. Not sure how this happened. My mistake.
| # See the License for the specific language governing permissions and | ||
| # limitations under the License. | ||
| # | ||
| """Audio hardware abstraction layer. |
There was a problem hiding this comment.
I can see from a diff that there aren't many changes here (compared to the normal version), however it does mean we need to maintain two files and keep them in sync. Is it worth adding a flag to Dinkum to know if we're running in a CI environment, then including these changes directly in Dinkum?
There was a problem hiding this comment.
I thought about that. For now it is only 2 lines which needed to be commented out. We do not need to keep these in sync as it is simple a dummy layer so either way is fine. It just seemed building this without having to modify dinkum proper was a win. I can see it either way.
| # Copy dinkum code and virtual environment | ||
| COPY --from=build-dinkum --chown=mycroft:mycroft /opt/mycroft-dinkum/.venv/ /opt/mycroft-dinkum/.venv/ | ||
| COPY --chown=mycroft:mycroft mycroft-dinkum/ /opt/mycroft-dinkum/ | ||
| RUN sed -i 's|worktree\s\+=.*|worktree = ../|' /opt/mycroft-dinkum/.git/config |
There was a problem hiding this comment.
Currently failing here for me, but it's getting late in the day so I haven't dug into why.
There was a problem hiding this comment.
I have no idea why? It works here but I have a bunch of stuff cached. I'll look into it today.
|
I will fix some of the above as noted then assign the ticket back to Gez to get the Allure stuff integrated. |
…existing system.
Description
See README file in vk_tests/ subdirectory
If needed follow up with as much detail as required.
Type of PR
If your PR fits more than one category, there is a high chance you should submit more than one PR. Please consider this carefully before opening the PR.
Either delete those that do not apply, or add an x between the square brackets like so:
- [x]Testing
How can someone reviewing this PR test that it is working properly? Is there appropriate test coverage for this change?
bash vk_tests/build-and-run-vk-tests.sh
Documentation
Does documentation for this already exist? Are there docstrings on all the public methods you added or modified? Have these been updated?
See README
CLA
I am an employee