action #34303
closedFix test for undocumented backend variables
0%
Description
Observation¶
In os-autoinst we have a test t/04-check_vars_docu.t which should check that all backend variables are properly documented however the test seems to not catch cases like https://github.com/os-autoinst/os-autoinst/pull/940#issuecomment-378567024 . The corresponding travis check states that everything is ok but when executing the test manually I get quite some errors but the exit code is 0:
$ perl t/04-check_vars_docu.t
still missing explanation for backend IKVM variable IPMI_HOSTNAME
still missing explanation for backend IKVM variable IPMI_HW
still missing explanation for backend IKVM variable IPMI_PASSWORD
still missing explanation for backend IKVM variable IPMI_USER
still missing explanation for backend IPMI variable IPMI_HOSTNAME
still missing explanation for backend IPMI variable IPMI_HW
still missing explanation for backend IPMI variable IPMI_PASSWORD
still missing explanation for backend IPMI variable IPMI_USER
still missing explanation for backend PVM variable ARCH
still missing explanation for backend PVM variable HDD_$hdd_num
still missing explanation for backend PVM variable HDD_$i
still missing explanation for backend PVM variable ISO
still missing explanation for backend PVM variable LPARID
still missing explanation for backend PVM variable NIC
still missing explanation for backend PVM variable VIOISO
still missing explanation for backend PVM variable VNC
still missing explanation for backend PVM variable WORKER_ID
still missing explanation for backend QEMU variable AUTO_INST
still missing explanation for backend QEMU variable BIOS
still missing explanation for backend QEMU variable HDDFORMAT
still missing explanation for backend QEMU variable HDDSIZEGB_$i
still missing explanation for backend QEMU variable OFW
still missing explanation for backend QEMU variable OVS_DEBUG
still missing explanation for backend QEMU variable QEMU_COMPRESS_LEVEL
still missing explanation for backend QEMU variable QEMU_COMPRESS_METHOD
still missing explanation for backend QEMU variable QEMU_MAX_BANDWIDTH
still missing explanation for backend QEMU variable RAIDLEVEL
still missing explanation for backend QEMU variable SCSICONTROLLER
still missing explanation for backend QEMU variable TAPDOWNSCRIPT
still missing explanation for backend QEMU variable TAPSCRIPT
still missing explanation for backend QEMU variable UEFI
still missing explanation for backend QEMU variable UEFI_BIOS
still missing explanation for backend QEMU variable VIRTIO_CONSOLE
still missing explanation for backend QEMU variable VNCKB
still missing explanation for backend QEMU variable WORKER_HOSTNAME
still missing explanation for backend SVIRT variable NUMDISKS
still missing explanation for backend SVIRT variable RAIDLEVEL
still missing explanation for backend VIRT variable QEMUCPUS
still missing explanation for backend VIRT variable QEMURAM
Suggestion¶
- t/04-check_vars_docu.t has some logic to provide a proper exit code based on if it found a mismatch. This should be checked
- It's annoying that doc/backend_vars.asciidoc is always updated when the "test" is executed locally. It should be made sure that tests do not modify files. Maybe add a command line switch to make that an option to update the documentation automatically
Updated by dheidler over 5 years ago
- Status changed from New to In Progress
- Assignee set to dheidler
Updated by dheidler over 5 years ago
- Status changed from In Progress to Feedback
Updated by okurz over 5 years ago
- Status changed from Resolved to Workable
As https://github.com/os-autoinst/os-autoinst/pull/1152 showed it looks like we still haven't reached the goal. The test isn't really "fixed" and finding the diff is even harder now that we have a separate file. We should really make it a working test, i.e. if a PR introduces a new undocumented backend variable then the test fails and also travis CI checks fail pointing to the necessary change, similar to tidy diffs.
Updated by ilausuch over 4 years ago
- Status changed from Workable to In Progress
- Assignee set to ilausuch
Updated by ilausuch over 4 years ago
Updated by livdywan over 4 years ago
- Status changed from In Progress to Feedback
PR got merged, addressing both the tests (by making them fatal) as well as gaps in documentation.
Updated by livdywan over 4 years ago
- Status changed from Feedback to Resolved
Suggestion¶
- t/04-check_vars_docu.t has some logic to provide a proper exit code based on if it found a mismatch. This should be checked
This part is covered.
- It's annoying that doc/backend_vars.asciidoc is always updated when the "test" is executed locally. It should be made sure that tests do not modify files. Maybe add a command line switch to make that an option to update the documentation automatically
A file doc/backend_vars.asciidoc.newvars is left behind, which unfortunately is neither temporary nor in .gitignore.
That said, this can probably be considered done.
Updated by okurz over 4 years ago
- Copied to action #71146: [os-autoinst][backends] Prevent undocumented test variables in backends / prevent use of external testapi methods added
Updated by okurz over 4 years ago
funny thing is that t/04-check_vars_docu.t only checks for variables like $bmwqemu::vars and not uses of get_var or get_required_var in our backends. Covered in #71146