search

Dasharo / dcu

Dasharo Configuration Utility
Apache License 2.0
2 stars 0 forks source link

Dcuc testing #13

Closed macpijan closed 13 hours ago

macpijan commented 2 months ago

@BeataZdunczyk @mkopec @philipandag Please use tests and implement new ones for new features. We are already missing tests for the most recently added feature.

Testing is documented in the README: https://github.com/Dasharo/dcu?tab=readme-ov-file#testing

As a followup, we need following improvements:

philipandag commented 2 months ago

@macpijan The tests for mac subcommand are in a weird situation, because we have not yet published a binary that supports them. To use the command, the binary has to have a valid gbe section with a valid checksum of the mac address but the binaries we have on cloud have the section filled with FF bytes, so the checksum test fails. Should I just prepare a rom that works and push it to dcu repo?

macpijan commented 2 months ago

The tests for mac subcommand are in a weird situation, because we have not yet published a binary that supports them.

NovaCustom MTL releases do not support this? I thought this was the tool for the non-GPU models, which have been released already?

philipandag commented 2 months ago

NovaCustom MTL releases do not support this? I thought this was the tool for the non-GPU models, which have been released already?

To work on these binaries the GBE section has to be first replaced with a valid, blank one. We have the gbe blob available to download in our repo: https://github.com/Dasharo/dasharo-blobs/blob/mtl-h/novacustom/v5x0tu/gbe.bin

The tests could download both the binary and gbe section and then replace it in the binary to perform tests.

mkopec commented 2 months ago

The iGPU releases shipped without a GbE region @macpijan . The blob was added to the repo after the release and DCU requires that blob to be in the binary.

We can inject the blob into the binary after the fact and provide it somewhere publicly (we probably don't want to replace the original binaries, but provide "full" binaries alongside them)

macpijan commented 2 months ago

The tests could download both the binary and gbe section and then replace it in the binary to perform tests.

That sounds ok to me.

philipandag commented 2 months ago

I have added tests for mac command. There is still the issue that approvals show absolute paths which will look different every time someone runs the approvals. Are we fixing that?

philipandag commented 2 months ago

We are using an old version of approve.bash. The new version allows for automatically accepting some differences as @macpijan suggested:

There is an option to allow for some diff (such as local paths): https://github.com/dannyben/approvals.bash?tab=readme-ov-file#allowing-some-difference

I have tried using this option with $PWD which caused sed, which is used to replace the allowed string with * , to crash. Then I tried passing escaped $PWD (${PWD//\//\\/}) but that just did not make any difference.

philipandag commented 2 weeks ago

With the latest commits a CI worflow was added. It was tested to work locally using https://github.com/nektos/act but I can see it does not on the remote. It needs some debugging.

philipandag commented 2 weeks ago

Looks like the CI is now running fine although the logs don't look like the tests are actually performed, like it ends just after downloading the files needed for the tests...