check name of action cannot be empty

Author: rhysd

action_metadata.go

@@ -236,8 +236,12 @@ func (c *LocalActionsCache) FindMetadata(spec string) (*ActionMetadata, bool, er
 	meta.file = f
 	meta.dir = dir
 
-	c.debug("New metadata parsed from action %s: %v", dir, &meta)
+	if meta.Name == "" {
+		c.writeCache(spec, nil) // Remember action was invalid
+		return nil, false, fmt.Errorf("name is required in action metadata %q", meta.Path())
+	}
 
+	c.debug("New metadata parsed from action %s: %v", dir, &meta)
 	c.writeCache(spec, &meta)
 	return &meta, false, nil
 }

action_metadata_test.go

@@ -74,7 +74,7 @@ func testCheckCachedFlag(t *testing.T, want, have bool) {
 	}
 }
 
-func TestLocalActionsFindMetadata(t *testing.T) {
+func TestLocalActionsFindMetadataOK(t *testing.T) {
 	testdir := filepath.Join("testdata", "action_metadata")
 	proj := &Project{testdir, nil}
 	c := NewLocalActionsCache(proj, nil)
@@ -295,27 +295,46 @@ func TestLocalActionsNullCache(t *testing.T) {
 // Error cases
 
 func TestLocalActionsBrokenMetadata(t *testing.T) {
+	tests := []struct {
+		spec string
+		want string
+	}{
+		{
+			spec: "./broken",
+			want: "could not parse action metadata",
+		},
+		{
+			spec: "./no_name",
+			want: "name is required in action metadata ",
+		},
+	}
+
 	proj := &Project{filepath.Join("testdata", "action_metadata"), nil}
 	c := NewLocalActionsCache(proj, nil)
-	m, cached, err := c.FindMetadata("./broken")
-	if err == nil {
-		t.Fatal("error was not returned", m)
-	}
-	if !strings.Contains(err.Error(), "could not parse action metadata") {
-		t.Fatal("unexpected error:", err)
-	}
-	testCheckCachedFlag(t, false, cached)
 
-	// Second try does not return error, but metadata is also nil not to show the same error from
-	// multiple rules.
-	m, cached, err = c.FindMetadata("./broken")
-	if err != nil {
-		t.Fatal("error was returned at second try", err)
-	}
-	if m != nil {
-		t.Fatal("metadata was not nil even if it does not exist", m)
+	for _, tc := range tests {
+		t.Run(tc.spec, func(t *testing.T) {
+			m, cached, err := c.FindMetadata(tc.spec)
+			if err == nil {
+				t.Fatal("error was not returned", m)
+			}
+			if !strings.Contains(err.Error(), tc.want) {
+				t.Fatalf("expected error message %q to contain %q", err, tc.want)
+			}
+			testCheckCachedFlag(t, false, cached)
+
+			// Second try does not return error, but metadata is also nil not to show the same error from
+			// multiple rules.
+			m, cached, err = c.FindMetadata(tc.spec)
+			if err != nil {
+				t.Fatal("error was returned at second try", err)
+			}
+			if m != nil {
+				t.Fatal("metadata was not nil even if it does not exist", m)
+			}
+			testCheckCachedFlag(t, true, cached)
+		})
 	}
-	testCheckCachedFlag(t, true, cached)
 }
 
 func TestLocalActionsDuplicateInputsOutputs(t *testing.T) {

testdata/action_metadata/no_name/action.yml

@@ -0,0 +1,6 @@
+author: 'rhysd <https://rhysd.github.io>'
+description: 'name is missing'
+
+runs:
+  using: 'node20'
+  main: index.js