jamke / qubes-completion Goto Github PK

Currently tests are not covering all commands and cases.
That should be improved. We need to generate and manually review tests for all commands.
The project includes automatic tool generating cases, but manual review and work is still required.

Any help with test will be appreciated.

Since Qubes 4.2 is officially out now there are some new tools now, for what could be in principle bash completion provided for. There are also some tools like qubesctl what have new options available.

I open this issues to collect to tools affected:

New tools introduced:

• qubes-fwupgmr
• qubes-vm-update
• qubes-guivm-session
• qubes-prepare-vm-kernel
• qubes-app-menu
• qubes-update-gui
• qubes-policy-lint
• qubes-policy-editor

Tools with new options or new possible arguments

• qubesctl
• qvm-features

Some command like qvm-shutdown or qvm-kill suggest dom0 as an target VM. This should not be the case.

Possible Solution:

• Remove dom0 each time by hand.
• Rework __get_qubes_list and add new input arguments options like running- for getting all running qubes but dom0

Context

There is a test suite, that is progressively being augmented to provide comprehensive regression testing coverage.

Overview

When I (or someone else!) add or modify completion commands, I want to make sure the modification don't break existing functionality.

Of course, contributors and maintainers are supposed to run the test suite (at least) before new releases are created, (ideally) before PRs are merged. In my experience, the green ticks that GitHub adds in PR when the test suite runs automatically, and the typical green badge in the README help making sure that the test suite failures are indeed detected early enough for their cause to be easy to track.

How

While a number of continuous integration services exist, creating a GitHub Action seems to me like a pretty good option (if not the best). Thoughts?

Considerations

• How often to trigger the test suite, given the time it takes to run. (Almost 20min on my machine, for reference.)

Overview

• Currently, testing currently depends on three tools provided by the BATS framework (docs):
  • bats-assert (CC0-1.0)
  • bats-file (CC0-1.0)
  • bats-support (CC0-1.0)
• Those need to be present in a specific path of the project before the existing test suite can be executed (test/test_helper/bats-*).
• Given their license, they could be included in the project

Why

• Remove the need for would-be contributors to fetch and add them themselves
• Open the possibility of running the test suite in a GitHub Action (I'll open a separate issue to discuss that)

How

Git submodules

Pros: Adding the three BATS repositories as git submodules makes it easier to keep track of which versions of the tools are currently in use, and eventually to update them.
Cons: However, it requires being aware of submodules when pulling the repository. (Mitigation: a note can be added to the Tests section of the documentation.)

Copies

A simple copy of the BATS tools can also be included with the project.

Pros: Simple. Those become part of the project like any other file, and no special setup is needed before tests can be run.
Cons: It is less obvious which version is used (which may or not matter).

Use local -r for constant variables.
Should improve code and allow to avoid possible mistakes.

We can significantly improve using salt in qubes when we provide further completion the qubesctl command.

Some basic completion for the first non-optional argument should at least be:

• top.enable
• top.disable
• top.enabled
• state.highstate
• state.apply

This will be easy to implement and could be easily expanded.

The really interesting part however would be to implement path completion the the salt commands that want one or more top files as an input.

Unfortunately the target file's default directory is within /srv/ which is only readable for the root user. Therefore I suppose we can only path complete after becoming root.

Furthermore after the top file[s] additional optional argument can be provided like log-level=debug

Therefor the full completion is something like

qubesctl [--qubes-options] salt-command path-to-top-file[s] [--salt-specific-arguments] 

References:

• https://www.qubes-os.org/doc/salt/
• https://docs.saltproject.io/en/latest/topics/using_salt.html
• In dom0 run salt-call --doc

I volunteer to tackle this but I wanted open an issue first to discuss.

Update test to fit current version with improvements and typo fixes.
Otherwise tests are not passed.

Overview

While the intent seems clear, the current use of code from the bash-completion project doesn't quite follow the license rules about re-use. (In a gist: keep the copyright statements, add your own to any file that contains a significant piece of code that someone else contributed to writing. This doesn't pretend to be legal speech, I am not a lawyer.)

Specifically src/qubes-completion.sh is missing the copyright statements that correspond to the piece of code derived from bash-completion.

The THIRDPARTY file contains a disclaimer about the origin of the code. However, that is not quite sufficient to fulfill the license obligations. (Standalone disclaimers like that are generally used for libraries or assets that are redistributed unmodified, but that is not the case here.)

Why bother?

I want this project to thrive, and I think all authors should get proper credit for their work. It turns out that's exactly what the copyright rules surrounding licensed do, in an unambiguous manner. The same rules will ensure that projects derived from this one give proper credit to its author(s). 🙂

Fix (option 1)

In this case, the situation is fairly straightforward and the fix easy:

• src/qubes-completion.sh contains code derived from code that was released under the GPL-2.0-or-later (according to the THIRDPARTY file)
• that the copyright headers that are included in THIRDPARTY should appear in src/qubes-completion.sh
• once that's done, the THIRDPARTY file becomes redundant

Fix (alternative)

If identifying specifically the piece of code that's derived from bash-completion is desired, that function could be extracted to a separate file, that would contains all the copyright statements.

At release time, the release artifact could be created by concatenating all source files. (That does add some tooling to the repo, for example a Makefile, which may or not be desirable.)