address feedback

Signed-off-by: Pranav Gaikwad <[email protected]>

docs/testrunner.md

@@ -24,7 +24,7 @@ A tests file contains three fields _rulesPath_, _providers_ and _tests_ at the t
 rulesPath: "/optional/path/to/rules/file"
 providers:
   - name: "go"
-    dataPath: "/path/to/data/for/this/provider"
+    dataPath: "/path/to/test/data/for/this/provider"
 tests:
   - ruleID: "rule-id-for-this-test"
     testCases:
@@ -54,7 +54,7 @@ _name_ is the name of the provider to which the config applies to, and _dataPath
 
 > Note that _dataPath_ must be relative to the directory in which tests file exists.
 
-If all tests under a _ruleset_ share values of _providers_ field (e.g. they use common data directory in all tests for a given provider), this config can also be defined at ruleset level under a special file `testing-config.yaml`. In that case, config present in this file will apply to all tests in that directory. A more specific config for a certain file can still be defined in the tests file. In that case, values in the tests file will take precedance over values at the _ruleset_ level.
+If all tests under a _ruleset_ share values of _providers_ field (e.g. they use common data directory in all tests for a given provider), this config can also be defined at ruleset level under a special file `testing-config.yaml`. In that case, the config present in this file will apply to all tests in that directory. A more specific config for a certain file can still be defined in the tests file. In that case, values in the tests file will take precedance over values at the _ruleset_ level.
 
 See an example of ruleset level config in [../pkg/testing/examples/ruleset/testing-config.yaml](../pkg/testing/examples/ruleset/testing-config.yaml).
 

pkg/container/container.go

@@ -190,7 +190,9 @@ func (c *container) Run(ctx context.Context, opts ...Option) error {
 		args = append(args, c.entrypointArgs...)
 	}
 	if c.reproducerCmd != nil {
-		*c.reproducerCmd = fmt.Sprintf("%s %s", c.containerRuntimeBin, strings.Join(args, " "))
+		reproducer := strings.ReplaceAll(strings.Join(args, " "), " --rm", "")
+		*c.reproducerCmd = fmt.Sprintf("%s %s",
+			c.containerRuntimeBin, reproducer)
 	}
 	cmd := exec.CommandContext(ctx, c.containerRuntimeBin, args...)
 	errBytes := &bytes.Buffer{}

pkg/testing/examples/ruleset/test-data/python/requirements.txt

@@ -4,16 +4,13 @@ charset-normalizer==3.2.0
 docstring-to-markdown==0.12
 google-auth==2.23.0
 idna==3.4
-jedi==0.19.0
 kubernetes==28.1.0
 oauthlib==3.2.2
 parso==0.8.3
 pluggy==1.3.0
 pyasn1==0.5.0
 pyasn1-modules==0.3.0
 python-dateutil==2.8.2
-python-lsp-jsonrpc==1.1.1
-python-lsp-server==1.8.0
 PyYAML==6.0.1
 requests==2.31.0
 requests-oauthlib==1.3.1

pkg/testing/result.go

@@ -38,7 +38,7 @@ func PrintSummary(w io.WriteCloser, results []Result) {
 		}
 		summaryByRules[result.RuleID].total += 1
 		tcSummary.total += 1
-		if len(result.FailureReasons) == 0 {
+		if result.Passed {
 			summaryByRules[result.RuleID].passed += 1
 			tcSummary.passed += 1
 		}

pkg/testing/runner.go

@@ -67,6 +67,7 @@ var defaultProviderConfig = []provider.Config{
 				ProviderSpecificConfig: map[string]interface{}{
 					"lspServerName":                 "generic",
 					provider.LspServerPathConfigKey: "/root/go/bin/gopls",
+					"lspServerArgs":                 []string{},
 					"dependencyProviderPath":        "/usr/bin/golang-dependency-provider",
 				},
 			},
@@ -81,6 +82,7 @@ var defaultProviderConfig = []provider.Config{
 				ProviderSpecificConfig: map[string]interface{}{
 					"lspServerName":                 "pylsp",
 					provider.LspServerPathConfigKey: "/usr/local/bin/pylsp",
+					"lspServerArgs":                 []string{},
 					"workspaceFolders":              []string{},
 					"dependencyFolders":             []string{},
 				},
@@ -242,6 +244,7 @@ func runWorker(wg *sync.WaitGroup, inChan chan workerInput, outChan chan []Resul
 				logFile.Close()
 				continue
 			}
+			// we already know in this group, all tcs have same params, use any
 			analysisParams := tests[0].TestCases[0].AnalysisParams
 			// write provider settings file
 			volumes, err := ensureProviderSettings(tempDir, input.opts.RunLocal, input.testsFile, baseProviderConfig, analysisParams)
@@ -429,24 +432,11 @@ func ensureProviderSettings(tempDirPath string, runLocal bool, testsFile TestsFi
 	// depending on whether we run locally, or in a container, we will either use local paths or mounted paths
 	switch {
 	case runLocal:
-		// when running locally, we use the paths as-is
 		for _, override := range testsFile.Providers {
+			// when running locally, we use the paths as-is
 			dataPath := filepath.Join(filepath.Dir(testsFile.Path), filepath.Clean(override.DataPath))
-			for idx := range baseProviders {
-				base := &baseProviders[idx]
-				if base.Name == override.Name {
-					initConf := &base.InitConfig[0]
-					base.ContextLines = 100
-					initConf.AnalysisMode = params.Mode
-					switch base.Name {
-					case "python", "go", "nodejs":
-						initConf.ProviderSpecificConfig["workspaceFolders"] = []string{dataPath}
-					default:
-						initConf.Location = dataPath
-					}
-					final = append(final, *base)
-				}
-			}
+			final = append(final,
+				getMergedProviderConfig(override.Name, baseProviders, params, dataPath, tempDirPath)...)
 		}
 	default:
 		// in containers, we need to make sure we only mount unique path trees
@@ -478,22 +468,10 @@ func ensureProviderSettings(tempDirPath string, runLocal bool, testsFile TestsFi
 			volumes[filepath.Join(filepath.Dir(testsFile.Path), uniquePath)] = path.Join("/data", uniquePath)
 		}
 		for _, override := range testsFile.Providers {
-			mountedDataPath := path.Join("/data", filepath.Clean(override.DataPath))
-			for idx := range baseProviders {
-				base := &baseProviders[idx]
-				base.ContextLines = 100
-				if base.Name == override.Name {
-					initConf := &base.InitConfig[0]
-					initConf.AnalysisMode = params.Mode
-					switch base.Name {
-					case "python", "go", "nodejs":
-						initConf.ProviderSpecificConfig["workspaceFolders"] = []string{mountedDataPath}
-					default:
-						initConf.Location = mountedDataPath
-					}
-					final = append(final, *base)
-				}
-			}
+			// when running in the container, we use the mounted path
+			dataPath := filepath.Join("/data", filepath.Clean(override.DataPath))
+			final = append(final,
+				getMergedProviderConfig(override.Name, baseProviders, params, dataPath, "/shared")...)
 		}
 	}
 	content, err := json.Marshal(final)
@@ -507,6 +485,47 @@ func ensureProviderSettings(tempDirPath string, runLocal bool, testsFile TestsFi
 	return volumes, nil
 }
 
+// getMergedProviderConfig for a given provider in the tests file, find a base provider config and
+// merge values as per precedance (values in tests file take precedance)
+func getMergedProviderConfig(name string, baseConfig []provider.Config, params AnalysisParams, dataPath string, outputPath string) []provider.Config {
+	merged := []provider.Config{}
+	for idx := range baseConfig {
+		base := &baseConfig[idx]
+		base.ContextLines = 100
+		if base.Name == name {
+			initConf := &base.InitConfig[0]
+			if params.Mode != "" {
+				initConf.AnalysisMode = params.Mode
+			}
+			switch base.Name {
+			// languages enabled via generic provide use workspaceFolders instead of location
+			// we also enable detailed logging for different providers
+			case "python":
+				initConf.ProviderSpecificConfig["workspaceFolders"] = []string{dataPath}
+				// log things in the output directory for debugging
+				lspArgs, ok := initConf.ProviderSpecificConfig["lspServerArgs"].([]string)
+				if ok {
+					initConf.ProviderSpecificConfig["lspServerArgs"] = append(lspArgs,
+						"--log-file", path.Join(outputPath, "python-server.log"), "-vv")
+				}
+			case "go":
+				initConf.ProviderSpecificConfig["workspaceFolders"] = []string{dataPath}
+				lspArgs, ok := initConf.ProviderSpecificConfig["lspServerArgs"].([]string)
+				if ok {
+					initConf.ProviderSpecificConfig["lspServerArgs"] = append(lspArgs,
+						"--logfile", path.Join(outputPath, "go-server.log"), "-vv")
+				}
+			case "nodejs":
+				initConf.ProviderSpecificConfig["workspaceFolders"] = []string{dataPath}
+			default:
+				initConf.Location = dataPath
+			}
+			merged = append(merged, *base)
+		}
+	}
+	return merged
+}
+
 func groupTestsByAnalysisParams(tests []Test) [][]Test {
 	grouped := map[string]map[string]*Test{}
 	for _, t := range tests {