-
Notifications
You must be signed in to change notification settings - Fork 5
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
Update Deploy based on Mr. Cava's observations in loading and running the Test Suite #38
Comments
Thanks @gitwjr we will check and review. Especially, we fully agree with Zachs comment "..., it may make sense to just describe Linux setup and then have your offshoot WSL2 document callout the quick WSL2 specifics. Though I recognize this may be an overly simplified view of the world from my point of view." This was out initial intention to support only Linux. I will go through Zachs comments together with @FritzHeiden and make proposal for changes. |
@louaybassbouss |
@gitwjr fine for me to close, @FritzHeiden wdyt? |
I don't think we have such an overview yet.
Indeed this is a complaint. If the OF is moved into the same docker image as the test runner than hopefully that will reduce the number of opportunities to make a mistake.
I think QUICKSTART fixes this.
This overlaps with one of my comments in #41 (comment). There are several different configurations for Windows, Docker Desktop vs Docker in WSL2. Docker in WSL2 in Windows is almost the same as Linux except potentially for some IP address voodoo to ensure the test runner is visible on the network and the test runner and the OF can talk to each other.
I agree that WSL2 should be split out more explicitly.
No comment.
Yes.
Very much yes.
I think this was a vestige of the W3C WPT tests and is overtaken by events.
Yes.
This should be overtaken by moving the OF into the docker image but otherwise I agree. I use python virtual environments to run the OF.
No comment. |
I think running the tests on the DUT and running the recordings in the OF are two separate tasks, so combining the TR and OF would bloat the installation unnecessarily. I wouldn't want to install the TR if I just want to analyse a recording and vice versa. Additionally this may lead to a situation where there are problems with the TR part of the image, while the user only wants to use the OF.
I agree aswell, will work on this.
Yes, this should be a fairly simple addition to the documentation.
Yes, will be added.
Makes sense, will be added aswell.
This shouldn't be necessary when using the docker image.
Will have to investigate on that
Using the docker image eliminates this and any python related issue that may occur. |
Warnings on the size / time for downloading content are addressed in #73 |
@FritzHeiden has addressed one or two of these points already and I've split the remainder out to separate issues. |
@louaybassbouss
Please look over the report from Zach detailing his experiences loading and running the Test Suite and update the Deploy and/or identify anything you think is better addressed by Eurofins.
From Zach:
Okay I had some time this evening and did a pass here. Stopping because I have to set up the camera to continue and don't have time at the moment, but hope to finish the full setup test this week.
In the meantime, thought it would be good to send what I have so far starting with the DPCTF Deploy:
• Meta Notes
o Outside of being told "hey this is what you need to run" it's not clear what all is being run and why different components or technologies are used (docker, hostname, etc). Is there a good system/architecture overview document that can help people better understand what is all going on and how the build steps are pulling everything together?
o I mainly raise this because debugging why something is going wrong during the setup is going to be incredibly difficult for non-developers in my opinion.
• Setup Environment
o Is there more background available on desired support environments? The combination of Linux and Windows commands with various (on windows do this) comments is a bit odd reading through. Ultimately doesn't prevent function but might make sense to perform a wording cleanup pass.
o Given Microsoft's hard push of WSL2, it may make sense to just describe Linux setup and then have your offshoot WSL2 document callout the quick WSL2 specifics. Though I recognize this may be an overly simplified view of the world from my point of view.
o My personal environment has shifted to Windows 11 with development through WSL2, so that is how all my testing was done. Couple notes on the WSL2 Setup document
None of the PATH stuff is really applicable assuming the frameworks are cloned into the WSL2 image
Recommend just linking to Windows setup WSL document instead of trying to repeat as the information is already subtly out of date: https://learn.microsoft.com/en-us/windows/wsl/install
Docker install coverage is a good idea and the text as it is, is nice and simple
• Create Image
o The separation of test runner target and tests branch target not immediately obvious, might be worth restating why the separation occurs.
o Otherwise, script wrapper worked just fine
• Import Content
o Might suggest a data size warning for the import command, either in the README or prior to the script run. Mainly to avoid people putting this in a build pipeline and constantly pulling large files.
o Rerunning import the script doesn't appear to check for existence before re-downloading, might be a good idea again given the size
• Hosts
o Hosts modified fine, note that WSL2 its easiest to modify the Windows Hosts file
o Really unclear why so many names are needed though, can this be explained somewhere
• Start the Container / Run Tests
o Startup is just fine, might be nice to have the tests print "Runner ready" on command line since there are a few long-pauses during startup
o Tests able to run successfully locally and remotely (used my home network dns to point device at desktop)
Then the starting comments on Observation Framework:
• Similar Linux vs Windows environment comments
• Python Packages
o Since this installs python packages and is reliant on specific versions I would recommend adding python virtual environments to the install steps
If that isn't added, I would recommend removing the force pip upgrade in the install script
o Environment note
I'm on Ubuntu 22.04 which only goes back to Python3.7 because of libssl incompatibility. opencv-python=4.5.2.52 does not have a wheel for 3.10 so install fails
Upgrading the opencv-python=4.5.4.60 enables the first 3.10 support, alternatively using Python3.7 works with the original package (good python virtual environment case)
o If you require Python >= 3.6 in the readme and just move to a python requirements.txt file that would probably eliminate the need for the install script
The text was updated successfully, but these errors were encountered: