From c56d93d07104b7c934d58a0ccf2b004f39f83d01 Mon Sep 17 00:00:00 2001 From: Andreas Humenberger Date: Wed, 5 Jun 2024 11:42:48 +0200 Subject: [PATCH 1/2] Update Symflower from v36800 to v37153 to make use of a fix for "symflower test" The fix makes sure that coverage is still reported even if there are failing tests. --- tools/symflower.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/symflower.go b/tools/symflower.go index 9cc4bc17..8e08b212 100644 --- a/tools/symflower.go +++ b/tools/symflower.go @@ -91,7 +91,7 @@ func (*symflower) CheckVersion(logger *log.Logger, binaryPath string) (err error } // SymflowerVersionRequired holds the version of Symflower required for this revision of the evaluation. -const SymflowerVersionRequired = "36800" +const SymflowerVersionRequired = "37153" // RequiredVersion returns the required version of the tool. func (*symflower) RequiredVersion() string { From 30113f2956a0c92aeb6236bb8fa8176a7b175def Mon Sep 17 00:00:00 2001 From: Andreas Humenberger Date: Wed, 5 Jun 2024 09:42:54 +0200 Subject: [PATCH 2/2] fix, Do not ignore coverage count if there are failing tests Fixes #158 --- evaluate/repository.go | 3 +- language/golang/language.go | 31 ++++++++++++++------ language/golang/language_test.go | 42 +++++++++++++++++++++++---- language/java/language.go | 11 +++++-- language/java/language_test.go | 42 +++++++++++++++++++++++---- language/language.go | 2 +- language/testing/Language_mock_gen.go | 21 ++++++++++---- model/symflower/symflower_test.go | 3 +- 8 files changed, 124 insertions(+), 31 deletions(-) diff --git a/evaluate/repository.go b/evaluate/repository.go index 15dee1d5..906ba730 100644 --- a/evaluate/repository.go +++ b/evaluate/repository.go @@ -54,7 +54,8 @@ func Repository(logger *log.Logger, resultPath string, model evalmodel.Model, la repositoryAssessment.Add(assessments) repositoryAssessment.Award(metrics.AssessmentKeyResponseNoError) - coverage, err := language.Execute(log, testDataPath) + coverage, ps, err := language.Execute(log, testDataPath) + problems = append(problems, ps...) if err != nil { problems = append(problems, pkgerrors.WithMessage(err, filePath)) diff --git a/language/golang/language.go b/language/golang/language.go index 852f5724..f23cbd57 100644 --- a/language/golang/language.go +++ b/language/golang/language.go @@ -5,6 +5,7 @@ import ( "os" "path/filepath" "regexp" + "strconv" "strings" pkgerrors "github.com/pkg/errors" @@ -74,14 +75,10 @@ func (l *Language) TestFramework() (testFramework string) { return "" } -var languageGoNoTestsMatch = regexp.MustCompile(`(?m)^DONE (\d+) tests.*in (.+?)$`) -var languageGoCoverageMatch = regexp.MustCompile(`(?m)^coverage: (\d+\.?\d+)% of statements`) -var languageGoNoCoverageMatch = regexp.MustCompile(`(?m)^coverage: \[no statements\]$`) +var languageGoTestsErrorMatch = regexp.MustCompile(`DONE (\d+) tests, (\d+) failure`) // Execute invokes the language specific testing on the given repository. -func (l *Language) Execute(logger *log.Logger, repositoryPath string) (coverage uint64, err error) { - coverageFilePath := filepath.Join(repositoryPath, "coverage.json") - +func (l *Language) Execute(logger *log.Logger, repositoryPath string) (coverage uint64, problems []error, err error) { commandOutput, err := util.CommandWithResult(context.Background(), logger, &util.Command{ Command: []string{ "go", @@ -92,9 +89,10 @@ func (l *Language) Execute(logger *log.Logger, repositoryPath string) (coverage Directory: repositoryPath, }) if err != nil { - return 0, pkgerrors.WithMessage(pkgerrors.WithStack(err), commandOutput) + return 0, problems, pkgerrors.WithMessage(pkgerrors.WithStack(err), commandOutput) } + coverageFilePath := filepath.Join(repositoryPath, "coverage.json") commandOutput, err = util.CommandWithResult(context.Background(), logger, &util.Command{ Command: []string{ tools.SymflowerPath, "test", @@ -106,8 +104,23 @@ func (l *Language) Execute(logger *log.Logger, repositoryPath string) (coverage Directory: repositoryPath, }) if err != nil { - return 0, pkgerrors.WithMessage(pkgerrors.WithStack(err), commandOutput) + testSummary := languageGoTestsErrorMatch.FindStringSubmatch(commandOutput) + if len(testSummary) > 0 { + if failureCount, e := strconv.Atoi(testSummary[2]); e != nil { + return 0, problems, pkgerrors.WithStack(e) + } else if failureCount > 0 { + // If there are test failures, then this is just a soft error since we still are able to receive coverage data. + problems = append(problems, pkgerrors.WithMessage(pkgerrors.WithStack(err), commandOutput)) + } + } else { + return 0, problems, pkgerrors.WithMessage(pkgerrors.WithStack(err), commandOutput) + } + } + + coverage, err = language.CoverageObjectCountOfFile(coverageFilePath) + if err != nil { + return 0, problems, pkgerrors.WithMessage(pkgerrors.WithStack(err), commandOutput) } - return language.CoverageObjectCountOfFile(coverageFilePath) + return coverage, problems, nil } diff --git a/language/golang/language_test.go b/language/golang/language_test.go index d46ecfa3..63bd9ab8 100644 --- a/language/golang/language_test.go +++ b/language/golang/language_test.go @@ -64,9 +64,10 @@ func TestLanguageExecute(t *testing.T) { RepositoryPath string RepositoryChange func(t *testing.T, repositoryPath string) - ExpectedCoverage uint64 - ExpectedError error - ExpectedErrorText string + ExpectedCoverage uint64 + ExpectedProblemTexts []string + ExpectedError error + ExpectedErrorText string } validate := func(t *testing.T, tc *testCase) { @@ -89,7 +90,12 @@ func TestLanguageExecute(t *testing.T) { if tc.Language == nil { tc.Language = &Language{} } - actualCoverage, actualError := tc.Language.Execute(logger, repositoryPath) + actualCoverage, actualProblems, actualError := tc.Language.Execute(logger, repositoryPath) + + require.Equal(t, len(tc.ExpectedProblemTexts), len(actualProblems), "the number of expected problems need to match the number of actual problems") + for i, expectedProblemText := range tc.ExpectedProblemTexts { + assert.ErrorContains(t, actualProblems[i], expectedProblemText) + } if tc.ExpectedError != nil { assert.ErrorIs(t, actualError, tc.ExpectedError) @@ -107,7 +113,8 @@ func TestLanguageExecute(t *testing.T) { RepositoryPath: filepath.Join("..", "..", "testdata", "golang", "plain"), - ExpectedCoverage: 0, + ExpectedCoverage: 0, + ExpectedErrorText: "exit status 1", }) t.Run("With test file", func(t *testing.T) { @@ -132,6 +139,31 @@ func TestLanguageExecute(t *testing.T) { ExpectedCoverage: 1, }) + validate(t, &testCase{ + Name: "Failing tests", + + RepositoryPath: filepath.Join("..", "..", "testdata", "golang", "light"), + RepositoryChange: func(t *testing.T, repositoryPath string) { + require.NoError(t, os.WriteFile(filepath.Join(repositoryPath, "simpleIfElse_test.go"), []byte(bytesutil.StringTrimIndentations(` + package light + + import ( + "testing" + ) + + func TestSimpleIfElse(t *testing.T) { + simpleIfElse(1) // Get some coverage... + t.Fail() // ...and then fail. + } + `)), 0660)) + }, + + ExpectedCoverage: 1, + ExpectedProblemTexts: []string{ + "exit status 1", // Test execution fails. + }, + }) + validate(t, &testCase{ Name: "Syntax error", diff --git a/language/java/language.go b/language/java/language.go index 5274a75d..21c9e807 100644 --- a/language/java/language.go +++ b/language/java/language.go @@ -89,7 +89,7 @@ func (l *Language) TestFramework() (testFramework string) { var languageJavaCoverageMatch = regexp.MustCompile(`Total coverage (.+?)%`) // Execute invokes the language specific testing on the given repository. -func (l *Language) Execute(logger *log.Logger, repositoryPath string) (coverage uint64, err error) { +func (l *Language) Execute(logger *log.Logger, repositoryPath string) (coverage uint64, problems []error, err error) { coverageFilePath := filepath.Join(repositoryPath, "coverage.json") commandOutput, err := util.CommandWithResult(context.Background(), logger, &util.Command{ Command: []string{ @@ -102,8 +102,13 @@ func (l *Language) Execute(logger *log.Logger, repositoryPath string) (coverage Directory: repositoryPath, }) if err != nil { - return 0, pkgerrors.WithMessage(pkgerrors.WithStack(err), commandOutput) + return 0, nil, pkgerrors.WithMessage(pkgerrors.WithStack(err), commandOutput) } - return language.CoverageObjectCountOfFile(coverageFilePath) + coverage, err = language.CoverageObjectCountOfFile(coverageFilePath) + if err != nil { + return 0, nil, pkgerrors.WithMessage(pkgerrors.WithStack(err), commandOutput) + } + + return coverage, nil, nil } diff --git a/language/java/language_test.go b/language/java/language_test.go index b4b91fec..966ec3f9 100644 --- a/language/java/language_test.go +++ b/language/java/language_test.go @@ -130,9 +130,10 @@ func TestLanguageExecute(t *testing.T) { RepositoryPath string RepositoryChange func(t *testing.T, repositoryPath string) - ExpectedCoverage uint64 - ExpectedError error - ExpectedErrorText string + ExpectedCoverage uint64 + ExpectedProblemTexts []string + ExpectedError error + ExpectedErrorText string } validate := func(t *testing.T, tc *testCase) { @@ -155,7 +156,12 @@ func TestLanguageExecute(t *testing.T) { if tc.Language == nil { tc.Language = &Language{} } - actualCoverage, actualError := tc.Language.Execute(logger, repositoryPath) + actualCoverage, actualProblems, actualError := tc.Language.Execute(logger, repositoryPath) + + require.Equal(t, len(tc.ExpectedProblemTexts), len(actualProblems), "the number of expected problems need to match the number of actual problems") + for i, expectedProblemText := range tc.ExpectedProblemTexts { + assert.ErrorContains(t, actualProblems[i], expectedProblemText) + } if tc.ExpectedError != nil { assert.ErrorIs(t, actualError, tc.ExpectedError) @@ -173,7 +179,8 @@ func TestLanguageExecute(t *testing.T) { RepositoryPath: filepath.Join("..", "..", "testdata", "java", "plain"), - ExpectedCoverage: 0, // TODO Let the test case identify and error that there are no test files (needs to be implemented in `symflower test`). https://github.com/symflower/eval-dev-quality/issues/35 + ExpectedCoverage: 0, + ExpectedErrorText: "exit status 1", }) t.Run("With test file", func(t *testing.T) { @@ -201,6 +208,31 @@ func TestLanguageExecute(t *testing.T) { ExpectedCoverage: 1, }) + validate(t, &testCase{ + Name: "Failing tests", + + RepositoryPath: filepath.Join("..", "..", "testdata", "java", "light"), + RepositoryChange: func(t *testing.T, repositoryPath string) { + javaTestFilePath := filepath.Join(repositoryPath, "src/test/java/com/eval/SimpleIfElseSymflowerTest.java") + require.NoError(t, os.MkdirAll(filepath.Dir(javaTestFilePath), 0755)) + require.NoError(t, os.WriteFile(javaTestFilePath, []byte(bytesutil.StringTrimIndentations(` + package com.eval; + + import org.junit.jupiter.api.*; + + public class SimpleIfElseSymflowerTest { + @Test + public void simpleIfElse() { + int actual = SimpleIfElse.simpleIfElse(1); // Get some coverage... + Assertions.assertEquals(true, false); // ... and then fail. + } + } + `)), 0660)) + }, + + ExpectedCoverage: 3, + }) + validate(t, &testCase{ Name: "Syntax error", diff --git a/language/language.go b/language/language.go index b690df34..45d806a9 100644 --- a/language/language.go +++ b/language/language.go @@ -23,7 +23,7 @@ type Language interface { TestFramework() (testFramework string) // Execute invokes the language specific testing on the given repository. - Execute(logger *log.Logger, repositoryPath string) (coverage uint64, err error) + Execute(logger *log.Logger, repositoryPath string) (coverage uint64, problems []error, err error) } // Languages holds a register of all languages. diff --git a/language/testing/Language_mock_gen.go b/language/testing/Language_mock_gen.go index 53437ae4..7c1bf004 100644 --- a/language/testing/Language_mock_gen.go +++ b/language/testing/Language_mock_gen.go @@ -13,7 +13,7 @@ type MockLanguage struct { } // Execute provides a mock function with given fields: logger, repositoryPath -func (_m *MockLanguage) Execute(logger *log.Logger, repositoryPath string) (uint64, error) { +func (_m *MockLanguage) Execute(logger *log.Logger, repositoryPath string) (uint64, []error, error) { ret := _m.Called(logger, repositoryPath) if len(ret) == 0 { @@ -21,8 +21,9 @@ func (_m *MockLanguage) Execute(logger *log.Logger, repositoryPath string) (uint } var r0 uint64 - var r1 error - if rf, ok := ret.Get(0).(func(*log.Logger, string) (uint64, error)); ok { + var r1 []error + var r2 error + if rf, ok := ret.Get(0).(func(*log.Logger, string) (uint64, []error, error)); ok { return rf(logger, repositoryPath) } if rf, ok := ret.Get(0).(func(*log.Logger, string) uint64); ok { @@ -31,13 +32,21 @@ func (_m *MockLanguage) Execute(logger *log.Logger, repositoryPath string) (uint r0 = ret.Get(0).(uint64) } - if rf, ok := ret.Get(1).(func(*log.Logger, string) error); ok { + if rf, ok := ret.Get(1).(func(*log.Logger, string) []error); ok { r1 = rf(logger, repositoryPath) } else { - r1 = ret.Error(1) + if ret.Get(1) != nil { + r1 = ret.Get(1).([]error) + } } - return r0, r1 + if rf, ok := ret.Get(2).(func(*log.Logger, string) error); ok { + r2 = rf(logger, repositoryPath) + } else { + r2 = ret.Error(2) + } + + return r0, r1, r2 } // Files provides a mock function with given fields: logger, repositoryPath diff --git a/model/symflower/symflower_test.go b/model/symflower/symflower_test.go index 127fc740..c52a1bf0 100644 --- a/model/symflower/symflower_test.go +++ b/model/symflower/symflower_test.go @@ -62,8 +62,9 @@ func TestModelGenerateTestsForFile(t *testing.T) { } metricstesting.AssertAssessmentsEqual(t, tc.ExpectedAssessment, actualAssessment) - actualCoverage, err := tc.Language.Execute(logger, repositoryPath) + actualCoverage, actualProblems, err := tc.Language.Execute(logger, repositoryPath) require.NoError(t, err) + require.Empty(t, actualProblems) assert.Equal(t, tc.ExpectedCoverage, actualCoverage) }) }