Bug 1183516

Summary:	yast-bootloader proposal code too generic and not sufficiently auto-tested
Product:	[openSUSE] openSUSE Tumbleweed	Reporter:	Stefan Hundhammer <shundhammer>
Component:	YaST2	Assignee:	YaST Team <yast-internal>
Status:	CONFIRMED ---	QA Contact:	Jiri Srain <jsrain>
Severity:	Normal
Priority:	P5 - None	CC:	jreidinger, mrmazda
Version:	Current
Target Milestone:	---
Hardware:	All
OS:	openSUSE Tumbleweed
URL:	https://trello.com/c/umsAMUg2
Whiteboard:
Found By:	---	Services Priority:
Business Priority:		Blocker:	---
Marketing QA Status:	---	IT Deployment:	---

Description Stefan Hundhammer 2021-03-15 09:46:30 UTC

This is fallout of bug #1183500 which turned out to be a duplicate of bug #1179482:

A new symbol for the YaST bootloader proposal was introduced, but only half-way: There was the hyperlink in the generated HTML, but no function counterpart.

This should not happen. This was a totally avoidable problem.

Worse, it's hard to find if you are not that familiar with the code: A simple "grep" for the symbol that threw an exception does not lead you to the code that actually does something because a symbol like "disable_update_nvram" is taken apart and processed based on string fragments.

Since we keep talking so much about test coverage, why don't we have a test that covers this?

Why don't we have a test that iterates over ALL the proposal fragments and checks if there is a method for each hyperlink so this kind of problem doesn't keep coming back?

Worse, since the strings are copied & pasted again and again, a simple typo will already lead to a crash.

And speaking of which, this kind of problem should definitely not be a showstopper for an installation / upgrade as happened here. This shouldn't happen in the first place because we clearly should have a test that checks it, but if something happens, it should not crash the installation and thus throw away all data and work that the user put into it.

Comment 1 Stefan Hundhammer 2021-03-15 09:51:27 UTC

Relevant code locations:

Generating the HTML:

https://github.com/yast/yast-bootloader/blob/master/src/lib/bootloader/grub2base.rb#L425


The array holding all href targets:

https://github.com/yast/yast-bootloader/blob/master/src/lib/bootloader/proposal_client.rb#L73

(checking them all should be another test)


Taking the clicked link target apart:

https://github.com/yast/yast-bootloader/blob/master/src/lib/bootloader/proposal_client.rb#L105


The code that actually handles the hyperlink:

https://github.com/yast/yast-bootloader/blob/master/src/lib/bootloader/proposal_client.rb#L358-L387