search

libremesh / lime-packages

LibreMesh packages configuring OpenWrt for wireless mesh networking
https://libremesh.org/
GNU Affero General Public License v3.0
277 stars 96 forks source link

Unit testing update #1027

Closed ilario closed 1 year ago

ilario commented 1 year ago

On https://github.com/libremesh/lime-packages/pull/1012#issuecomment-1509146128 @spiccinini wrote that we should update the tests for working with the new LibreMesh code, as the tests were broken with the change from supporting OpenWrt 19.07 to OpenWrt 21 and 22.

I managed to fix some issues, but there are two that are still there:

1) The new OpenWrt style moves the list of the ports in the br-lan from a named interface to an unnamed device. In the tests, this stupid @device[0] thing does not get found:

Error → packages/lime-system/tests/test_lime_network.lua @ 77
LiMe Network tests test network.configure() with only lime.proto.lan
packages/lime-system/tests/test_lime_network.lua:100: attempt to index a nil value

stack traceback:
    packages/lime-system/tests/test_lime_network.lua:100: in function <packages/lime-system/tests/test_lime_network.lua:77>

The error comes from this line: https://github.com/ilario/lime-packages/blob/c3e1b44764094b893e07b3a2bd62c964c7cbe5ee/packages/lime-system/tests/test_lime_network.lua#L100

On the hardware seems that there is no problem, as that @device[0] interface gets created by OpenWrt. In the tests, maaaaaaybe, the interface does not get added because there is no device yet? So that we should add a uci add network device somewhere? Where is the right place?

2) There is some issue with the LibreRouter testing. I don't think it is related to the LibreRouter, but I tried to update its files to recent OpenWrt style and something didn't work. No idea what is happening here.

Failure → tests/test_lime_config_device.lua @ 17
LiMe Config tests test lime-config for a LibreRouter device #librerouter
tests/test_lime_config_device.lua:44: Expected objects to be equal.
Passed in:
(nil)
Expected:
(string) 'lan1'

stack traceback:
    tests/test_lime_config_device.lua:44: in function <tests/test_lime_config_device.lua:17>

The error comes from this line: https://github.com/ilario/lime-packages/blob/c3e1b44764094b893e07b3a2bd62c964c7cbe5ee/tests/test_lime_config_device.lua#L44

ilario commented 1 year ago

As you can see in the automated unit test log ( https://github.com/libremesh/lime-packages/actions/runs/5101071877/jobs/9169703391 ) there are a lot of other creepy messages, but interestingly they do not result in failed tests. I paste here the verbose output of the tests:

$ LUA_ENABLE_LOGGING=1 ./run_tests 
●●●●●●●●●●●●●●Good signature of firmware_latest.json
●Bad signature of firmware_latest.json
●●Downloading the firmware from ../20.xx/targets/ar71xx/generic/test-board-upgrade.sh
Error: the sha256 does not match
●Downloading the firmware from ../20.xx/targets/ar71xx/generic/test-board-upgrade.sh
Firmware downloaded ok
●●●●●●●●●cp: can't stat '/usr/share/lime/configs/lime-node': No such file or directory
●●●killall: firstbootwizard: no process killed
cp: can't stat '/tmp/wireless-temp': No such file or directory
●●●●●●●●●@packages/lime-hwd-openwrt-wan/files/usr/lib/lua/lime/hwd/openwrt_wan.lua:59 detect_hardware WAN interface: wan
●@packages/lime-hwd-openwrt-wan/files/usr/lib/lua/lime/hwd/openwrt_wan.lua:61 detect_hardware WAN interface not found
●@packages/lime-hwd-openwrt-wan/files/usr/lib/lua/lime/hwd/openwrt_wan.lua:61 detect_hardware WAN interface not found
●●●●●●●●●●●●●●●●●●Applying generic configs:
 libremap
Done applying generic configs.
●Applying generic configs:
 invalid
 Error on generic config uci_set: invalid.settings.foo=bar
Done applying generic configs.
●Applying generic configs:
 invalid_syntax
 Error on generic config uci_set: this is bad s!ntax=='1000'
Done applying generic configs.
●Copying assets:
  collectd (community/collectd.conf)
 Error copying asset 'community/collectd.conf': file not found.
Done copying assets.
●Copying assets:
  collectd (node/collectd.conf)
Done copying assets.
●Running assets on ATCONFIG :
  dropbear (node/dropbear.sh)
Done running assets.
●Running assets on ATCONFIG :
  dropbear (community/dropbear.sh)
 Warning: the asset '/tmp/lime-assets/community/dropbear.sh': returnen non zero status.
Done running assets.
●Running assets on ATCONFIG :
  dropbear (comunity/i_dont_exist.sh)
 Error running asset 'comunity/i_dont_exist.sh': file not found .
Done running assets.
●●●●INVALID main_ipv4_address 10.13.0.0/16 IDENTICAL TO RESERVED NETWORK ADDRESS. USING 10.13.255.254/16
●INVALID main_ipv4_address 10.13.0.0/16 IDENTICAL TO RESERVED NETWORK ADDRESS. USING 10.13.255.254/16
◼●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●migrate-wifi-modes: migration finished, now run lime-config
●migrate-wifi-modes: migration finished, now run lime-config
●migrate-wifi-modes: migration finished, now run lime-config
●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●Preserving libremesh minimal config.
List of files that are being preserved:
    etc/init.d/safe_upgrade_auto_reboot
    etc/rc.d/S11safe_upgrade_auto_reboot
    etc/safe_upgrade_auto_reboot_confirm_timeout_s
    tmp/tmp.GGkbaf/foo
●Not preserving config.
List of files that are being preserved:
    etc/init.d/safe_upgrade_auto_reboot
    etc/rc.d/S11safe_upgrade_auto_reboot
    etc/safe_upgrade_auto_reboot_confirm_timeout_s
●Preserving from archive
List of files that are being preserved:
    etc/init.d/safe_upgrade_auto_reboot
    etc/rc.d/S11safe_upgrade_auto_reboot
    etc/safe_upgrade_auto_reboot_confirm_timeout_s
    proc/version
●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●sh: iw: not found
●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●iw dev wlan1-mesh scan freq 2412 2462
iw dev wlan2-mesh scan freq 5180 5240
●iw dev wlan1-mesh scan freq 2412 2462
iw dev wlan2-mesh scan freq 5180 5240
●●●●●●●●●Mac-config: lime-3a95394b4b6e544e99361a980ee75db3
Clearing wireless config...
Clearing network config...
Disabling odhcpd
Cleaning dnsmasq
Disabling 6relayd...
@packages/lime-hwd-openwrt-wan/files/usr/lib/lua/lime/hwd/openwrt_wan.lua:59 detect_hardware WAN interface: wan
...ages/lime-system/files/usr/lib/lua/lime/wireless.lua:54: attempt to index local 'devModes' (a nil value)
stack traceback:
    ...ckages/lime-system/files/usr/lib/lua/lime/config.lua:214: in function <...ckages/lime-system/files/usr/lib/lua/lime/config.lua:214>
    ...ages/lime-system/files/usr/lib/lua/lime/wireless.lua:54: in function 'is5Ghz'
    ...ages/lime-system/files/usr/lib/lua/lime/wireless.lua:41: in function 'scandevices'
    ...ages/lime-system/files/usr/lib/lua/lime/wireless.lua:134: in function <...ages/lime-system/files/usr/lib/lua/lime/wireless.lua:128>
    [C]: in function 'xpcall'
    ...ckages/lime-system/files/usr/lib/lua/lime/config.lua:214: in function 'main'
    tests/test_lime_config_device.lua:41: in function <tests/test_lime_config_device.lua:17>
    [C]: in function 'xpcall'
    /usr/local/share/lua/5.1/busted/core.lua:178: in function 'safe'
    /usr/local/share/lua/5.1/busted/init.lua:40: in function 'executor'
    /usr/local/share/lua/5.1/busted/core.lua:312: in function </usr/local/share/lua/5.1/busted/core.lua:312>
    ...
    /usr/local/share/lua/5.1/busted/block.lua:155: in function 'execute'
    /usr/local/share/lua/5.1/busted/init.lua:7: in function 'executor'
    /usr/local/share/lua/5.1/busted/core.lua:312: in function </usr/local/share/lua/5.1/busted/core.lua:312>
    [C]: in function 'xpcall'
    /usr/local/share/lua/5.1/busted/core.lua:178: in function 'safe'
    /usr/local/share/lua/5.1/busted/core.lua:312: in function 'execute'
    /usr/local/share/lua/5.1/busted/execute.lua:58: in function 'execute'
    /usr/local/share/lua/5.1/busted/runner.lua:197: in function </usr/local/share/lua/5.1/busted/runner.lua:11>
    ...r/local/lib/luarocks/rocks/busted/2.0.0-1/bin/busted:3: in main chunk
    [C]: ?
network.scandevices.owrt_network_interface_parser found ifname br-lan
network.scandevices.owrt_network_interface_parser found ifname wan
ls: /sys/class/net/wan/lower*: No such file or directory
...kages/lime-system/files/usr/lib/lua/lime/network.lua:254: attempt to index field '?' (a nil value)
stack traceback:
    ...ckages/lime-system/files/usr/lib/lua/lime/config.lua:214: in function <...ckages/lime-system/files/usr/lib/lua/lime/config.lua:214>
    [C]: in function 'foreach'
    ...kages/lime-system/files/usr/lib/lua/lime/network.lua:326: in function 'scandevices'
    ...kages/lime-system/files/usr/lib/lua/lime/network.lua:355: in function <...kages/lime-system/files/usr/lib/lua/lime/network.lua:346>
    [C]: in function 'xpcall'
    ...ckages/lime-system/files/usr/lib/lua/lime/config.lua:214: in function 'main'
    tests/test_lime_config_device.lua:41: in function <tests/test_lime_config_device.lua:17>
    [C]: in function 'xpcall'
    /usr/local/share/lua/5.1/busted/core.lua:178: in function 'safe'
    /usr/local/share/lua/5.1/busted/init.lua:40: in function 'executor'
    /usr/local/share/lua/5.1/busted/core.lua:312: in function </usr/local/share/lua/5.1/busted/core.lua:312>
    ...
    /usr/local/share/lua/5.1/busted/block.lua:155: in function 'execute'
    /usr/local/share/lua/5.1/busted/init.lua:7: in function 'executor'
    /usr/local/share/lua/5.1/busted/core.lua:312: in function </usr/local/share/lua/5.1/busted/core.lua:312>
    [C]: in function 'xpcall'
    /usr/local/share/lua/5.1/busted/core.lua:178: in function 'safe'
    /usr/local/share/lua/5.1/busted/core.lua:312: in function 'execute'
    /usr/local/share/lua/5.1/busted/execute.lua:58: in function 'execute'
    /usr/local/share/lua/5.1/busted/runner.lua:197: in function </usr/local/share/lua/5.1/busted/runner.lua:11>
    ...r/local/lib/luarocks/rocks/busted/2.0.0-1/bin/busted:3: in main chunk
    [C]: ?
Configuring system...
Let uhttpd listen on IPv4/IPv6
Applying generic configs:
 uhttpd_https
Done applying generic configs.
Copying assets:
Done copying assets.
Running assets on ATFIRSTBOOT :
Done running assets.
Running assets on ATCONFIG :
Done running assets.
◼●●●●●
282 successes / 2 failures / 0 errors / 0 pending : 4.348614 seconds

Failure → packages/lime-system/tests/test_lime_network.lua @ 77
LiMe Network tests test network.configure() with only lime.proto.lan
packages/lime-system/tests/test_lime_network.lua:100: Expected objects to be equal.
Passed in:
(nil)
Expected:
(string) 'eth99'

stack traceback:
    packages/lime-system/tests/test_lime_network.lua:100: in function <packages/lime-system/tests/test_lime_network.lua:77>

Failure → tests/test_lime_config_device.lua @ 17
LiMe Config tests test lime-config for a LibreRouter device #librerouter
tests/test_lime_config_device.lua:44: Expected objects to be equal.
Passed in:
(nil)
Expected:
(string) 'lan1'

stack traceback:
    tests/test_lime_config_device.lua:44: in function <tests/test_lime_config_device.lua:17>
spiccinini commented 1 year ago

Hey! Great work Ilario!!! I've done some improvements and also one fix.

The librerouter test is still failing, I started debugging it but not finished yet. The first step to fix is is to allow mocking the way that the lower interfaces are being detected because the ls command will not work as it is not being run in the real device, something like this:

+function network._get_lower(dev)
+    local lower_if_path = utils.unsafe_shell("ls -d /sys/class/net/" .. dev .. "/lower*")
+    local lower_if_table = utils.split(lower_if_path, "_")
+    return lower_if_table[#lower_if_table]:gsub("\n", "")
+end
+
 function network.scandevices()
        local devices = {}
        local switch_vlan = {}
@@ -239,19 +245,16 @@ function network.scandevices()
                end
                --! With DSA, the LAN ports are not anymore eth0.1 but lan1, lan2...
                if dev:match("^lan%d+$") then
-                       local lower_if_path = utils.unsafe_shell("ls -d /sys/class/net/" .. dev .. "/lower*")
-                       local lower_if_table = utils.split(lower_if_path, "_")
-                       local lower_if = lower_if_table[#lower_if_table]:gsub("\n", "")
+                       local lower_if = network._get_lower(dev)
                        devices[lower_if] = { nobridge = true }
                        devices[dev] = {}
                        utils.log( "network.scandevices.dev_parser found LAN port %s " ..
                                   "and marking %s as nobridge", dev, lower_if )
                end
                --! With DSA, the WAN is named wan. Copying the code from the lan case.
+
                if dev:match("^wan$") then
-                       local lower_if_path = utils.unsafe_shell("ls -d /sys/class/net/" .. dev .. "/lower*")
-                       local lower_if_table = utils.split(lower_if_path, "_")
-                       local lower_if = lower_if_table[#lower_if_table]:gsub("\n", "")
+                       local lower_if = network._get_lower(dev)
                        devices[lower_if] = { nobridge = true }
                        devices[dev] = {}
ilario commented 1 year ago

Thanks @spiccinini !!!!! That patch you propose in your comment looks great, are you going to include it as a commit or should I do that? Thanks!

spiccinini commented 1 year ago

Tests pass now! So I added some more fixes, please review!

I did not tested it in a device, I don't have any until next week

ilario commented 1 year ago

Awesome work @spiccinini, plenty of fixes here! Thanks!!

Just one comment: in 5e47b7c79b1091d2d3205f8d4316e2d4cf3e0944 you replace eth string with lan one, but even in OpenWrt 22.03 there are still routers that use the eth naming. For example, the PlasmaCloud PA1200 that we used for the network at the Wireless BattleMesh v15: https://github.com/openwrt/openwrt/blob/openwrt-22.03/target/linux/ipq40xx/base-files/etc/board.d/02_network#L23

that is even changing to ethernet1 and ethernet2 in the future OpenWrt 23.05: https://github.com/openwrt/openwrt/blob/openwrt-23.05/target/linux/ipq40xx/base-files/etc/board.d/02_network#L98

spiccinini commented 1 year ago

So the best thing would be to find a way to get the type of the interface without relying on the name, right?

ilario commented 1 year ago

I guess so...

It is a lot of work to do, but it is the right thing to do.

The dirty solution would be to just check for both eth and lan strings... Until when we complete the support for OpenWrt 22.03, and then working on the general solution.

codecov-commenter commented 1 year ago

Codecov Report

Merging #1027 (21bf277) into master (4cce5b5) will increase coverage by 1.42%. The diff coverage is 70.00%.

:exclamation: Current head 21bf277 differs from pull request most recent head c3ef5a8. Consider uploading reports for the commit c3ef5a8 to get more accurate results

:exclamation: Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more.

@@            Coverage Diff             @@
##           master    #1027      +/-   ##
==========================================
+ Coverage   77.84%   79.27%   +1.42%     
==========================================
  Files          52       53       +1     
  Lines        4388     4444      +56     
==========================================
+ Hits         3416     3523     +107     
+ Misses        972      921      -51
Impacted Files Coverage Δ
...-lime-utils/files/usr/lib/lua/lime-utils-admin.lua 57.14% <2.77%> (-35.59%) :arrow_down:
...es/lime-system/files/usr/lib/lua/lime/firewall.lua 64.00% <14.28%> (+19.26%) :arrow_up:
...us-lime-metrics/files/usr/lib/lua/lime-metrics.lua 86.36% <36.36%> (-6.23%) :arrow_down:
...kages/lime-system/files/usr/lib/lua/lime/utils.lua 85.22% <71.42%> (-0.58%) :arrow_down:
...e-proto-bmx7/files/usr/lib/lua/lime/proto/bmx7.lua 71.08% <78.57%> (+5.79%) :arrow_up:
...ges/lime-system/files/usr/lib/lua/lime/network.lua 78.19% <78.94%> (+2.85%) :arrow_up:
...-lime-utils/files/usr/lib/lua/lime/node_status.lua 83.51% <83.51%> (ø)
...wrt-wan/files/usr/lib/lua/lime/hwd/openwrt_wan.lua 100.00% <100.00%> (ø)
...oto-babeld/files/usr/lib/lua/lime/proto/babeld.lua 95.94% <100.00%> (+0.05%) :arrow_up:
...oto-batadv/files/usr/lib/lua/lime/proto/batadv.lua 96.36% <100.00%> (-0.52%) :arrow_down:
... and 5 more

... and 1 file with indirect coverage changes

:mega: We’re building smart automated test selection to slash your CI/CD build times. Learn more

ilario commented 1 year ago

The dirty solution would be to just check for both eth and lan strings... Until when we complete the support for OpenWrt 22.03, and then working on the general solution.

Just did this. @spiccinini can you merge?