Fix builtin discovery tools not being loaded if no platform is installed (#1428)

* Fix integration tests detected_boards fixture

* Fix builtin discoveries not loaded if no platform is installed

* [skip changelog] Add missing breaking change to UPGRADING.md

* [skip changelog] Fix board list test running on CI

* [skip changelog] Fix UPGRADING.md

Author: silvanocerza

arduino/cores/packagemanager/loader.go

@@ -683,6 +683,39 @@ func (pm *PackageManager) LoadDiscoveries() []*status.Status {
 	for _, platform := range pm.InstalledPlatformReleases() {
 		statuses = append(statuses, pm.loadDiscoveries(platform)...)
 	}
+	if st := pm.loadBuiltinDiscoveries(); len(st) > 0 {
+		statuses = append(statuses, st...)
+	}
+	return statuses
+}
+
+// loadDiscovery loads the discovery tool with id, if it cannot be found a non-nil status is returned
+func (pm *PackageManager) loadDiscovery(id string) *status.Status {
+	tool := pm.GetTool(id)
+	if tool == nil {
+		return status.Newf(codes.FailedPrecondition, tr("discovery not found: %s"), id)
+	}
+	toolRelease := tool.GetLatestInstalled()
+	if toolRelease == nil {
+		return status.Newf(codes.FailedPrecondition, tr("discovery not installed: %s"), id)
+	}
+	discoveryPath := toolRelease.InstallDir.Join(tool.Name).String()
+	d, err := discovery.New(id, discoveryPath)
+	if err != nil {
+		return status.Newf(codes.FailedPrecondition, tr("creating discovery: %s"), err)
+	}
+	pm.discoveryManager.Add(d)
+	return nil
+}
+
+// loadBuiltinDiscoveries loads the discovery tools that are part of the builtin package
+func (pm *PackageManager) loadBuiltinDiscoveries() []*status.Status {
+	statuses := []*status.Status{}
+	for _, id := range []string{"builtin:serial-discovery", "builtin:mdns-discovery"} {
+		if st := pm.loadDiscovery(id); st != nil {
+			statuses = append(statuses, st)
+		}
+	}
 	return statuses
 }
 
@@ -705,23 +738,9 @@ func (pm *PackageManager) loadDiscoveries(release *cores.PlatformRelease) []*sta
 	//
 	// If both indexed and unindexed properties are found the unindexed are ignored
 	for _, id := range discoveryProperties.ExtractSubIndexLists("required") {
-		tool := pm.GetTool(id)
-		if tool == nil {
-			statuses = append(statuses, status.Newf(codes.FailedPrecondition, tr("discovery not found: %s"), id))
-			continue
-		}
-		toolRelease := tool.GetLatestInstalled()
-		if toolRelease == nil {
-			statuses = append(statuses, status.Newf(codes.FailedPrecondition, tr("discovery not installed: %s"), id))
-			continue
-		}
-		discoveryPath := toolRelease.InstallDir.Join(tool.Name).String()
-		d, err := discovery.New(id, discoveryPath)
-		if err != nil {
-			statuses = append(statuses, status.Newf(codes.FailedPrecondition, tr("creating discovery: %s"), err))
-			continue
+		if st := pm.loadDiscovery(id); st != nil {
+			statuses = append(statuses, st)
 		}
-		pm.discoveryManager.Add(d)
 	}
 
 	discoveryIDs := discoveryProperties.FirstLevelOf()

arduino/cores/packagemanager/loader_test.go

@@ -141,7 +141,9 @@ func TestLoadDiscoveries(t *testing.T) {
 	})
 
 	errs := packageManager.LoadDiscoveries()
-	require.Len(t, errs, 0)
+	require.Len(t, errs, 2)
+	require.Equal(t, errs[0].Message(), "discovery not found: builtin:serial-discovery")
+	require.Equal(t, errs[1].Message(), "discovery not found: builtin:mdns-discovery")
 	discoveries := packageManager.DiscoveryManager().IDs()
 	require.Len(t, discoveries, 1)
 	require.Contains(t, discoveries, "arduino:ble-discovery")
@@ -154,7 +156,9 @@ func TestLoadDiscoveries(t *testing.T) {
 	})
 
 	errs = packageManager.LoadDiscoveries()
-	require.Len(t, errs, 0)
+	require.Len(t, errs, 2)
+	require.Equal(t, errs[0].Message(), "discovery not found: builtin:serial-discovery")
+	require.Equal(t, errs[1].Message(), "discovery not found: builtin:mdns-discovery")
 	discoveries = packageManager.DiscoveryManager().IDs()
 	require.Len(t, discoveries, 2)
 	require.Contains(t, discoveries, "arduino:ble-discovery")
@@ -169,7 +173,9 @@ func TestLoadDiscoveries(t *testing.T) {
 	})
 
 	errs = packageManager.LoadDiscoveries()
-	require.Len(t, errs, 0)
+	require.Len(t, errs, 2)
+	require.Equal(t, errs[0].Message(), "discovery not found: builtin:serial-discovery")
+	require.Equal(t, errs[1].Message(), "discovery not found: builtin:mdns-discovery")
 	discoveries = packageManager.DiscoveryManager().IDs()
 	require.Len(t, discoveries, 3)
 	require.Contains(t, discoveries, "arduino:ble-discovery")
@@ -186,7 +192,9 @@ func TestLoadDiscoveries(t *testing.T) {
 	})
 
 	errs = packageManager.LoadDiscoveries()
-	require.Len(t, errs, 0)
+	require.Len(t, errs, 2)
+	require.Equal(t, errs[0].Message(), "discovery not found: builtin:serial-discovery")
+	require.Equal(t, errs[1].Message(), "discovery not found: builtin:mdns-discovery")
 	discoveries = packageManager.DiscoveryManager().IDs()
 	require.Len(t, discoveries, 3)
 	require.Contains(t, discoveries, "arduino:ble-discovery")

docs/UPGRADING.md

@@ -4,6 +4,62 @@ Here you can find a list of migration guides to handle breaking changes between
 
 ## 0.19.0
 
+### `board list` command JSON output change
+
+The `board list` command JSON output has been changed quite a bit, from:
+
+```
+$ arduino-cli board list --format json
+[
+  {
+    "address": "/dev/ttyACM1",
+    "protocol": "serial",
+    "protocol_label": "Serial Port (USB)",
+    "boards": [
+      {
+        "name": "Arduino Uno",
+        "fqbn": "arduino:avr:uno",
+        "vid": "0x2341",
+        "pid": "0x0043"
+      }
+    ],
+    "serial_number": "954323132383515092E1"
+  }
+]
+```
+
+to:
+
+```
+$ arduino-cli board list --format json
+[
+  {
+    "matching_boards": [
+      {
+        "name": "Arduino Uno",
+        "fqbn": "arduino:avr:uno"
+      }
+    ],
+    "port": {
+      "address": "/dev/ttyACM1",
+      "label": "/dev/ttyACM1",
+      "protocol": "serial",
+      "protocol_label": "Serial Port (USB)",
+      "properties": {
+        "pid": "0x0043",
+        "serialNumber": "954323132383515092E1",
+        "vid": "0x2341"
+      }
+    }
+  }
+]
+```
+
+The `boards` array has been renamed `matching_boards`, each contained object will now contain only `name` and `fqbn`.
+Properties that can be used to identify a board are now moved to the new `properties` object, it can contain any key
+name. `pid` and `vid` have been moved to `properties`, `serial_number` has been renamed `serialNumber` and moved to
+`properties`. The new `label` field is the name of the `port` if it should be displayed in a GUI.
+
 ### gRPC interface `DebugConfigRequest`, `UploadRequest`, `UploadUsingProgrammerRequest`, `BurnBootloaderRequest`, `DetectedPort` field changes
 
 `DebugConfigRequest`, `UploadRequest`, `UploadUsingProgrammerRequest` and `BurnBootloaderRequest` had their `port` field

i18n/data/en.po

@@ -2447,7 +2447,7 @@ msgstr "can't find latest release of %s"
 msgid "can't find main Sketch file in %s"
 msgstr "can't find main Sketch file in %s"
 
-#: arduino/cores/packagemanager/loader.go:749
+#: arduino/cores/packagemanager/loader.go:768
 msgid "can't find pattern for discovery with id %s"
 msgstr "can't find pattern for discovery with id %s"
 
@@ -2518,7 +2518,7 @@ msgstr "computing hash: %s"
 msgid "could not find a valid build artifact"
 msgstr "could not find a valid build artifact"
 
-#: arduino/cores/packagemanager/loader.go:721
+#: arduino/cores/packagemanager/loader.go:705
 msgid "creating discovery: %s"
 msgstr "creating discovery: %s"
 
@@ -2555,11 +2555,11 @@ msgstr "destination dir %s already exists, cannot install"
 msgid "discovery %[1]s process not started: %[2]w"
 msgstr "discovery %[1]s process not started: %[2]w"
 
-#: arduino/cores/packagemanager/loader.go:710
+#: arduino/cores/packagemanager/loader.go:696
 msgid "discovery not found: %s"
 msgstr "discovery not found: %s"
 
-#: arduino/cores/packagemanager/loader.go:715
+#: arduino/cores/packagemanager/loader.go:700
 msgid "discovery not installed: %s"
 msgstr "discovery not installed: %s"
 

i18n/rice-box.go

@@ -203,14 +203,12 @@ def detected_boards(run_command):
 
     detected_boards = []
     for port in json.loads(result.stdout):
-        for board in port.get("boards", []):
-            fqbn = board.get("FQBN")
-            if not fqbn:
-                continue
+        for board in port.get("matching_boards", []):
+            fqbn = board.get("fqbn")
             package, architecture, _id = fqbn.split(":")
             detected_boards.append(
                 Board(
-                    address=port.get("address"),
+                    address=port["port"]["address"],
                     fqbn=fqbn,
                     package=package,
                     architecture=architecture,

test/conftest.py

@@ -16,6 +16,9 @@
 from git import Repo
 import simplejson as json
 import semver
+import pytest
+
+from .common import running_on_ci
 
 
 gold_board = """
@@ -389,6 +392,7 @@
 """  # noqa: E501
 
 
+@pytest.mark.skipif(running_on_ci(), reason="VMs have no serial ports")
 def test_board_list(run_command):
     run_command("core update-index")
     result = run_command("board list --format json")
@@ -397,8 +401,8 @@ def test_board_list(run_command):
     ports = json.loads(result.stdout)
     assert isinstance(ports, list)
     for port in ports:
-        assert "protocol" in port
-        assert "protocol_label" in port
+        assert "protocol" in port["port"]
+        assert "protocol_label" in port["port"]
 
 
 def test_board_listall(run_command):