Skip to content
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

test suite for Solaar #1097

Closed
kloczek opened this issue Feb 28, 2021 · 27 comments
Closed

test suite for Solaar #1097

kloczek opened this issue Feb 28, 2021 · 27 comments
Labels
enhancement help wanted long term Unlikely to be implemented quickly

Comments

@kloczek
Copy link

kloczek commented Feb 28, 2021

Everyting is moving away from setuppy test to pytest so it will be good to prepare solaar to pytest as well :)

+ /usr/bin/python3 -Bm pytest -ra
=========================================================================== test session starts ============================================================================
platform linux -- Python 3.8.8, pytest-6.2.2, py-1.10.0, pluggy-0.13.1
rootdir: /home/tkloczko/rpmbuild/BUILD/Solaar-1.0.5
plugins: flaky-3.6.1, forked-1.3.0, shutil-1.7.0, virtualenv-1.7.0, asyncio-0.14.0, expect-1.1.0, pyfakefs-4.1.0, cov-2.11.1, mock-3.5.1, httpbin-1.0.0, xdist-2.2.1, flake8-1.0.7, hypothesis-6.3.3, timeout-1.4.2
collected 0 items

========================================================================== no tests ran in 0.08s ===========================================================================
@pfps
Copy link
Collaborator

pfps commented Feb 28, 2021

Solaar doesn't have a test suite at all, AFAICT.

If you are interested in setting up a pytest test suite for Solaar, help would be appreciated.

@pfps pfps changed the title 1.0.5: pytest based test suite fails (no units) test suite for Solaar (preferably using pytest) Feb 28, 2021
@kloczek
Copy link
Author

kloczek commented Feb 28, 2021

A yesh .. you are right however 'setup.py test` exit with 0 exit code :P

@rijnhard
Copy link
Contributor

rijnhard commented Mar 9, 2021

eeeek. well I've written a test structure similar to what I think will work here.

Can we debate this for a sec?
So I would almost not even bother with unit level tests except for particularly sensitive or obviously suited components.

I'd go with an approach similar to how the Polly.JS handles HTTP requests.
We should capture and make stubs of the communications coming from the devices and play that back to solaar and ensure that solaar responds or does what is expected.

This way we will guard against regressions very heavily.
I can probably do the primary structure, but the capturing and replaying (with time sensitivities) of the communications would be pretty tough for my skill set.

Thoughts?

@pfps
Copy link
Collaborator

pfps commented Mar 9, 2021

That sounds like the start of a way to go. But there is almost certainly a lot of work that needs to be done here.

There are a few issues that need to be considered.

Solaar uses udev so that will have to be faked as well. I think faking at a high level is (device availability) is OK.

Devices come and go so there needs to be a way to provide error returns for the I/O.

Solaar is multi-threaded and reads and writes from several devices so the I/O stubs need to include timing.

Solaar is mostly GUI-based so there needs to be some way of simulating GUI input.

As far as capturing the communications, Solaar has a debug mode that logs all I/O to the devices, including timing, so these logs can be mined to get interactions.

@rijnhard
Copy link
Contributor

I think it should be separated into phases, it will also be easier to find contributors for the different aspects.

For example we can fake the input by replaying the I/O logs against predefined expected scenarios without interacting with the gui at all.

GUI tests are its own kettle of fish, I'd almost keep those completely separate and just ensure that they trigger the state changes they are supposed to, completely separate from I/O device scenario testing.

I'm willing to help with the setup of the test framework so that we can do the I/O device scenario testing (mainly because I've done similar in other projects), but someone else will have to add the feedback mechanism for the I/O errors.

Gui tests on the other hand... not my forte.

@pfps
Copy link
Collaborator

pfps commented Mar 10, 2021

That sounds reasonable.

What counts as a success for an I/O test, then? Solaar's reaction to information from devices is to update its internal data structures. How will the test harness determine how internal structures have changed?

@rijnhard
Copy link
Contributor

we will need a dictionary of responses for the different devices, basically made from the captured logs/responses.

in the test solaar will issue X command, then based on the dictionary for the device we will reply with Y Response. If the command does not match anything in the dictionary it will fail.

Thats atleast the MVP in my head.

The upside is you will quickly pick up regressions due to unintentional changes,
but the downsides are that if you intentionally change a command you will need to recapture the device logs.

Is this a major problem? Do commands being issued for the same purpose to the same device change often?

@pfps
Copy link
Collaborator

pfps commented Mar 16, 2021

(Sorry for the delayed reply. I wrote a reply and then neglected to post it.)

The dictionary needs to be a bit sophisticated because there is a random section in the command. This section is at a known place and is echoed in the response so it should be possible to model this.

There also should be a way to inject asynchronous inputs from the devices and delay the response to commands as one of the problems in Solaar occurs when input from devices goes to the wrong consumer.

@rijnhard
Copy link
Contributor

rijnhard commented Mar 21, 2021 via email

@pfps
Copy link
Collaborator

pfps commented Mar 22, 2021

Excellent. Having this testing ability could have caught #1113

@FFY00
Copy link
Member

FFY00 commented Oct 12, 2021

See the proposal in #590, that way we can actually test the whole stack. Basically, we emulate the devices using the Linux UHID API. The drawback is that it cannot run in Github action as it does not give us access to /dev/uhid.

@pfps pfps added the long term Unlikely to be implemented quickly label Mar 12, 2022
@jeblad
Copy link
Contributor

jeblad commented Sep 1, 2022

With a lot of globals and semi globals, the code will be pretty hard to test. It will be more testing of implementation than business logic.

@pfps
Copy link
Collaborator

pfps commented Sep 1, 2022

The test suite would be more for regression testing than testing for actual correctness.

@pfps pfps changed the title test suite for Solaar (preferably using pytest) test suite for Solaar Nov 9, 2022
@pfps pfps closed this as not planned Won't fix, can't repro, duplicate, stale Jan 20, 2024
@MattHag
Copy link
Collaborator

MattHag commented Feb 10, 2024

I'd love to support this request and introduce tests for Solaar. Starting with unit tests of small components is the simplest way to get started.

With a lot of globals and semi globals, the code will be pretty hard to test. It will be more testing of implementation than business logic.

Such issues can be identified while writing unit tests and the code improved for better testability. This will be an ongoing project, which should lead to an easier implementation of integration tests in the end.

Suggestion to introduce tests

  1. Add GitHub action that triggers pytest tests for Linux on pull request
  2. Introduce unit tests (and simplify code that's hard to test)
    Steadily raises coverage and lowers the entry hurdle for new developers, as automated tests catch more and more issues.
  3. Introduce integration tests (hardware mocks etc.)
    Could ease development with faked hardware.
  4. Extend GitHub action to test on Windows and macOS

@pfps pfps reopened this Feb 10, 2024
@pfps
Copy link
Collaborator

pfps commented Feb 10, 2024

That sounds reasonable, except that the first step should probably be to set up some unit tests so that the pytest has something to do. Feel free to set up a couple of these. Is there a good way of setting up data to run the tests?

A problem is how to simulate hardware. Is there something that can do this for python, at least to respond to the messages that Solaar sends, even if it doesn't simulate the delays that come from sending actual messages. And then there is the issue that hardware sends spontaneous messages.

@MattHag
Copy link
Collaborator

MattHag commented Feb 10, 2024

A simple test case is sufficient to setup the CI and test it. I have a change ready.

Unit tests should not require any hardware and just test a function, a class or a module on its own. However, I have seen that most code is tightly coupled, which makes testing harder. But that's a good point to introduce tests and at the same time reduce coupling of code.

For End-to-end tests, which can be added at a later point need more thought and will require a hardware fake, that supports the most basic commands. With a nice abstraction of the OS specific commands, it could easier to achieve this.

@pfps
Copy link
Collaborator

pfps commented Feb 10, 2024

Solaar has several interfaces to the OS. One is to find devices. The main one is to send and receive HID++ commands. The others are for rules. The first one to simulate would probably be for HID++ commands.

pfps pushed a commit that referenced this issue Feb 10, 2024
MattHag added a commit to MattHag/Solaar that referenced this issue Mar 9, 2024
MattHag added a commit to MattHag/Solaar that referenced this issue Mar 9, 2024
MattHag added a commit to MattHag/Solaar that referenced this issue Mar 10, 2024
MattHag added a commit to MattHag/Solaar that referenced this issue Mar 10, 2024
pfps pushed a commit that referenced this issue Mar 10, 2024
Pull get_kind_from_index from class to module level and add tests.

Related #1097
pfps pushed a commit that referenced this issue Mar 10, 2024
pfps pushed a commit that referenced this issue Mar 10, 2024
* test: Test base product information

Related #1097

* Fix dependencies for gi
@pfps
Copy link
Collaborator

pfps commented Mar 20, 2024

This issue is about using pytest. As there is now a partial test suite using pytest this issue can be closed. Issues can be opened about missing parts of the test suite, accompanied by suggestions on how to proceed.

@pfps pfps closed this as completed Mar 20, 2024
MattHag added a commit to MattHag/Solaar that referenced this issue Sep 19, 2024
MattHag added a commit to MattHag/Solaar that referenced this issue Sep 19, 2024
Enforce a total coverage of 40% of the code.

Related pwr-Solaar#1097
MattHag added a commit to MattHag/Solaar that referenced this issue Sep 19, 2024
Create coverage.xml, upload it to GitHub CI and visualize with codecov.

Requires CODECOV_TOKEN in the GitHub CI project secrets.

Related pwr-Solaar#1097
MattHag added a commit to MattHag/Solaar that referenced this issue Sep 19, 2024
Create coverage.xml, upload it to GitHub CI and visualize with codecov.

Setup instruction:
- Install codecov for project
  https://github.com/settings/installations/55029514
- Add CODECOV_TOKEN in the GitHub CI project secrets

Related pwr-Solaar#1097
MattHag added a commit to MattHag/Solaar that referenced this issue Sep 19, 2024
MattHag added a commit to MattHag/Solaar that referenced this issue Sep 25, 2024
MattHag added a commit to MattHag/Solaar that referenced this issue Sep 25, 2024
Enforce a total coverage of 40% of the code.

Related pwr-Solaar#1097
MattHag added a commit to MattHag/Solaar that referenced this issue Sep 25, 2024
Create coverage.xml, upload it to GitHub CI and visualize with codecov.

Setup instruction:
- Install codecov for project
  https://github.com/settings/installations/55029514
- Add CODECOV_TOKEN in the GitHub CI project secrets

Related pwr-Solaar#1097
MattHag added a commit to MattHag/Solaar that referenced this issue Sep 25, 2024
MattHag added a commit to MattHag/Solaar that referenced this issue Sep 25, 2024
Create coverage.xml, upload it to GitHub CI and visualize with codecov.

Setup instruction:
- Install codecov for project
  https://github.com/settings/installations/55029514
- Add CODECOV_TOKEN in the GitHub CI project secrets

Related pwr-Solaar#1097
MattHag added a commit to MattHag/Solaar that referenced this issue Sep 25, 2024
MattHag added a commit to MattHag/Solaar that referenced this issue Sep 25, 2024
pfps pushed a commit that referenced this issue Oct 8, 2024
pfps pushed a commit that referenced this issue Oct 8, 2024
Enforce a total coverage of 40% of the code.

Related #1097
pfps pushed a commit that referenced this issue Oct 8, 2024
Create coverage.xml, upload it to GitHub CI and visualize with codecov.

Setup instruction:
- Install codecov for project
  https://github.com/settings/installations/55029514
- Add CODECOV_TOKEN in the GitHub CI project secrets

Related #1097
pfps pushed a commit that referenced this issue Oct 8, 2024
MattHag added a commit to MattHag/Solaar that referenced this issue Oct 10, 2024
pfps pushed a commit that referenced this issue Oct 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement help wanted long term Unlikely to be implemented quickly
Projects
None yet
Development

No branches or pull requests

6 participants