Bug 1033 - Implementation and enhancement of "Test API"
Summary: Implementation and enhancement of "Test API"
Status: CONFIRMED
Alias: None
Product: Libre-SOC's first SoC
Classification: Unclassified
Component: Source Code (show other bugs)
Version: unspecified
Hardware: Other Linux
: --- enhancement
Assignee: Luke Kenneth Casson Leighton
URL:
Depends on:
Blocks:
 
Reported: 2023-03-17 10:50 GMT by Luke Kenneth Casson Leighton
Modified: 2023-08-30 10:32 BST (History)
4 users (show)

See Also:
NLnet milestone: NLnet.2022-08-107.ongoing
total budget (EUR) for completion of task and all subtasks: 2500
budget (EUR) for this task, excluding subtasks' budget: 2500
parent task for budget allocation: 961
child tasks for budget allocation:
The table of payments (in EUR) for this task; TOML format:
jacob=2000 lkcl=500


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Luke Kenneth Casson Leighton 2023-03-17 10:50:53 GMT
add microwatt, cavatools, FPGA and "standard Makefile"
targets for running as raw binaries in a VM/native/FPGA.
related to NLnet Grant OPF ISA 2022-08-051 cavatools.

the purpose of the "Test" API is to make it possible to
compare unit test results automatically against different
implementations.
Comment 1 Jacob Lifshay 2023-08-24 06:40:11 BST
https://git.libre-soc.org/?p=libreriscv.git;a=commitdiff;h=a1ba910b1ac26158648b69d5cbd7556d9be3a6ef

important clarification -- afaict this task is adding new `StateRunner`
and `State` subclasses for FPGA/verilator/etc.
This task is *not* for creating a new framework or choosing an existing framework,
we already have one with implementations for pypowersim, nmigen simulation of the
libre-soc core, and `ExpectedState`. maybe also QEMU through GDB, icr.
Comment 2 Luke Kenneth Casson Leighton 2023-08-24 09:06:48 BST
(In reply to Jacob Lifshay from comment #1)
> https://git.libre-soc.org/?p=libreriscv.git;a=commitdiff;
> h=a1ba910b1ac26158648b69d5cbd7556d9be3a6ef
> 
> important clarification -- afaict this task is adding new `StateRunner`
> and `State` subclasses for FPGA/verilator/etc.

pretty much yes. and morphing the existing Test API which is a bit of
a botch job to be more generic.


> This task is *not* for creating a new framework or choosing an existing
> framework,

correct 

> we already have one with implementations for pypowersim, nmigen simulation
> of the
> libre-soc core, and `ExpectedState`.

correct

>  maybe also QEMU through GDB, icr.

QEMU can be run but not through the Test API, it is also on the TODO list
to be added *to* the Test API.

Makefiles are probably the high priority, for running (ssh) on
a POWER9 system.
Comment 3 Jacob Lifshay 2023-08-24 17:39:57 BST
(In reply to Luke Kenneth Casson Leighton from comment #2)
> Makefiles are probably the high priority, for running (ssh) on
> a POWER9 system.

that could also be used for testing on QEMU by treating it as a remote system you can ssh into.

for ssh, we need to have a cached ssh connection (one per test process is fine imo) so we don't try to connect for each test, that ssh connection should pass the appropriate options to require non-interactive login (if we run with pytest -n 24 do you really want to type your passphrase 24 times?!) and to cache the failure to use ssh (so it doesn't try to retry creating a ssh connection for every unit test, reporting a Python warning is a good idea tho) and have that failure just remove the ssh State-subclass from the results, not mark tests as failing, so people who can't login to a POWER9 server can still run unit tests.
Comment 4 Jacob Lifshay 2023-08-24 17:49:20 BST
(In reply to Jacob Lifshay from comment #3)
> and have that failure just remove the ssh State-subclass from the
> results, not mark tests as failing, so people who can't login to a POWER9
> server can still run unit tests.

perhaps have a USE_SSH env var or something that changes the warning to an error, for CI.

(CI not part of this task afaict)

for CI we'd want a *unprivileged*/sandboxed user on the talos so CI can ssh into it and not mess the whole system up if we end up with something broken running on CI.
Comment 5 Luke Kenneth Casson Leighton 2023-08-24 20:59:41 BST
(In reply to Jacob Lifshay from comment #3)

> for ssh, we need to have a cached ssh connection (one per test process is
> fine imo) 

naah.  setup and teardown override is perfectly fine.
do not make this more complex than it has to be.

if it DOES turn out to be a problem THEN as a SECOND step it
can be "improved". but please do not jump immedately to the
"optimised solution" wasting time especially on a fixed budget
on things that have not been demonstrated as necessary.

please be more brutal and pragmatic as a routine matter of course.


btw ssh under remote connection was solved a long time ago (20 years).

look up telnetbase.py https://bugs.python.org/issue708007
note that it includes a TelnetSSH class.



so we don't try to connect for each test, that ssh connection
> should pass the appropriate options to require non-interactive login (if we
> run with pytest -n 24 do you really want to type your passphrase 24 times?!)

Offically Not Our Problem.  absolutely 100% out of scope.
i repeat. if people have not set up ssh-agent to the target
machine or not put a passphrase (i NEVER do, precisely for
this reason) it is their lookout.


> and to cache the failure to use ssh (so it doesn't try to retry creating a

no.  under no circumstances waste time on solving problems that can be
solved by people not being stupid.

we have limited time and limited budget. getting "something that works
if you follow certain steps" (such as using an ssh key that has
no passphrase) is good enough.
Comment 6 Jacob Lifshay 2023-08-24 21:25:57 BST
(In reply to Luke Kenneth Casson Leighton from comment #5)
> > and to cache the failure to use ssh (so it doesn't try to retry creating a
> 
> no.  under no circumstances waste time on solving problems that can be
> solved by people not being stupid.

you're missing the point, which is that some people *don't have access* to a POWER9 system so no matter how not-stupid they are ssh-to-a-power9-system will fail, because they have none.
Comment 7 Jacob Lifshay 2023-08-24 21:58:30 BST
(In reply to Luke Kenneth Casson Leighton from comment #5)
> look up telnetbase.py https://bugs.python.org/issue708007
> note that it includes a TelnetSSH class.

that file's corrupted when I tried...

I think we should use something like:
https://github.com/ParallelSSH/parallel-ssh#single-host-client
it uses libssh[2] and seems quite easy to use.
Comment 8 Jacob Lifshay 2023-08-24 21:59:35 BST
(In reply to Jacob Lifshay from comment #7)
> that file's corrupted when I tried...

gzip: stdin: decompression OK, trailing garbage ignored
telnetlib/
telnetlib/telnetlib2.py
telnetlib/telnetlib.py
telnetlib/expectlib.py
tar: Child returned status 2
tar: Error is not recoverable: exiting now