diff --git a/.github/workflows/test-processor.yml b/.github/workflows/test-processor.yml new file mode 100644 index 00000000..111a5e7c --- /dev/null +++ b/.github/workflows/test-processor.yml @@ -0,0 +1,25 @@ +name: Test processor + +on: + push: + paths: + - "script/**" + +jobs: + test-processor: + runs-on: ubuntu-latest + steps: + - name: Checkout repository + uses: actions/checkout@v4 + + - name: Setup Go + uses: actions/setup-go@v5 + with: + go-version-file: "script/go.mod" + cache-dependency-path: "script/go.sum" + + - name: Install dependencies + run: make install_deps + + - name: Test processor + run: make test diff --git a/Makefile b/Makefile index 690bc039..09ac011c 100644 --- a/Makefile +++ b/Makefile @@ -8,6 +8,10 @@ build_processor: $(info Building processor...) @cd ./script && go build -v -o ../processor . +test: + $(info Running tests...) + @cd ./script && go test + bump_version: $(info Bumping version...) @$(eval LAST_TAG=$(shell git rev-list --tags='processor-*' --max-count=1 | xargs -r git describe --tags --match 'processor-*')) diff --git a/script/application.go b/script/application.go index 6959adfe..72a219bd 100644 --- a/script/application.go +++ b/script/application.go @@ -71,18 +71,18 @@ func (a *Application) Parse(issue github.Issue) { isProject := !a.Project.IsTeam && !a.Project.IsEvent - a.Project.Name = a.stringSection("Project name", IsPresent, IsRegularString) - a.Project.Description = a.stringSection("Short description", IsPresent, IsRegularString) - a.Project.Contributors = a.intSection("Number of team members/core contributors", IsPresent, IsRegularString) + a.Project.Name = a.stringSection("Project name", IsPresent, ParsePlainString) + a.Project.Description = a.stringSection("Short description", IsPresent, ParsePlainString) + a.Project.Contributors = a.intSection("Number of team members/core contributors", IsPresent, ParsePlainString) a.Project.HomeUrl = a.stringSection("Homepage URL", IsPresent, IsUrl) a.Project.RepoUrl = a.stringSection("Repository URL", IsUrl) - a.Project.LicenseType = a.stringSection("License type", When(isProject, IsPresent), IsRegularString) + a.Project.LicenseType = a.stringSection("License type", When(isProject, IsPresent), ParsePlainString) a.Project.LicenseUrl = a.stringSection("License URL", When(isProject, IsPresent), IsUrl) a.boolSection("Age confirmation", When(isProject, IsPresent), ParseCheckbox, When(isProject, IsChecked)) - a.Applicant.Name = a.stringSection("Name", IsPresent, IsRegularString) + a.Applicant.Name = a.stringSection("Name", IsPresent, ParsePlainString) a.Applicant.Email = a.stringSection("Email", IsPresent, IsEmail) - a.Applicant.Role = a.stringSection("Project role", IsPresent, IsProjectRole) + a.Applicant.Role = a.stringSection("Project role", IsPresent) a.Applicant.Id = *issue.User.ID a.stringSection("Profile or website", IsUrl) diff --git a/script/application_test.go b/script/application_test.go new file mode 100644 index 00000000..428cb795 --- /dev/null +++ b/script/application_test.go @@ -0,0 +1,156 @@ +package main + +import ( + "fmt" + "os" + "testing" +) + +func errNoProjectName(sectionTitle string) error { + return fmt.Errorf("%s: is missing project name", sectionTitle) +} + +func errIncomplete(sectionTitle string) error { + return fmt.Errorf("%s: was not completed for application", sectionTitle) +} + +func errEmpty(sectionTitle string) error { + return fmt.Errorf("%s: is empty", sectionTitle) +} + +func errMustBeChecked(sectionTitle string) error { + return fmt.Errorf("%s: must be checked", sectionTitle) +} + +func errInvalidAccountUrl(sectionTitle string) error { + return fmt.Errorf("%s: is invalid 1Password account URL", sectionTitle) +} + +func errContainsEmoji(sectionTitle string) error { + return fmt.Errorf("%s: cannot contain emoji characters", sectionTitle) +} + +func errParsingNumber(sectionTitle string) error { + return fmt.Errorf("%s: could not be parsed into a number", sectionTitle) +} + +func errInvalidUrl(sectionTitle string) error { + return fmt.Errorf("%s: is an invalid URL", sectionTitle) +} + +func TestApplication(t *testing.T) { + originalDir, err := os.Getwd() + if err != nil { + t.Fatalf("Failed to get current directory: %s", err) + } + + defer func() { + if err := os.Chdir(originalDir); err != nil { + t.Fatalf("Failed to change back to original directory: %s", err) + } + }() + + if err := os.Chdir("../"); err != nil { + t.Fatalf("Failed to change working directory: %s", err) + } + + testCases := []struct { + name string + expectedValid bool + expectedProblems []error + }{ + { + name: "project", + expectedValid: true, + }, + { + name: "team", + expectedValid: true, + }, + { + name: "event", + expectedValid: true, + }, + { + name: "empty-body", + expectedValid: false, + expectedProblems: []error{ + errIncomplete("Account URL"), + errIncomplete("Non-commercial confirmation"), + errIncomplete("Team application"), + errIncomplete("Event application"), + errIncomplete("Project name"), + errIncomplete("Short description"), + errIncomplete("Number of team members/core contributors"), + errIncomplete("Homepage URL"), + errIncomplete("Repository URL"), + errIncomplete("License type"), + errIncomplete("License URL"), + errIncomplete("Age confirmation"), + errIncomplete("Name"), + errIncomplete("Email"), + errIncomplete("Project role"), + errIncomplete("Profile or website"), + errIncomplete("Additional comments"), + errIncomplete("Can we contact you?"), + }, + }, + { + name: "no-responses", + expectedValid: false, + expectedProblems: []error{ + errNoProjectName("Application title"), + errEmpty("Account URL"), + errMustBeChecked("Non-commercial confirmation"), + errEmpty("Project name"), + errEmpty("Short description"), + errEmpty("Number of team members/core contributors"), + errEmpty("Homepage URL"), + errEmpty("License type"), + errEmpty("License URL"), + errMustBeChecked("Age confirmation"), + errEmpty("Name"), + errEmpty("Email"), + errEmpty("Project role"), + }, + }, + { + name: "examples-1", + expectedValid: false, + expectedProblems: []error{ + errNoProjectName("Application title"), + errInvalidAccountUrl("Account URL"), + errMustBeChecked("Non-commercial confirmation"), + errContainsEmoji("Project name"), + errParsingNumber("Number of team members/core contributors"), + errInvalidUrl("Homepage URL"), + }, + }, + } + + for _, tt := range testCases { + t.Run(tt.name, func(t *testing.T) { + application := Application{} + + if tt.expectedValid { + testIssueName = fmt.Sprintf("valid-%s", tt.name) + } else { + testIssueName = fmt.Sprintf("invalid-%s", tt.name) + } + + application.Parse(*getTestIssue()) + + if application.IsValid() != tt.expectedValid { + if tt.expectedValid { + t.Errorf("Test issue '%s' is invalid, expected valid", testIssueName) + } else { + t.Errorf("Test issue '%s' is valid, expected invalid", testIssueName) + } + } + + if !tt.expectedValid && !errSliceEqual(application.Problems, tt.expectedProblems) { + t.Errorf("Expected problems %v, got %v", tt.expectedProblems, application.Problems) + } + }) + } +} diff --git a/script/testing.go b/script/testing.go index 130ea650..bd66a56b 100644 --- a/script/testing.go +++ b/script/testing.go @@ -40,3 +40,32 @@ func getTestIssue() *github.Issue { return &issue } + +func errSliceEqual(a, b []error) bool { + if len(a) != len(b) { + return false + } + + countA := make(map[string]int) + countB := make(map[string]int) + + for _, err := range a { + countA[err.Error()]++ + } + + for _, err := range b { + countB[err.Error()]++ + } + + if len(countA) != len(countB) { + return false + } + + for k, v := range countA { + if countB[k] != v { + return false + } + } + + return true +} diff --git a/script/validator.go b/script/validator.go index fcf505cf..3bbb6ab9 100644 --- a/script/validator.go +++ b/script/validator.go @@ -17,8 +17,7 @@ var ( accountUrlRegex = regexp.MustCompile(`^(https?:\/\/)?[\w.-]+\.1password\.(com|ca|eu)\/?$`) urlRegex = regexp.MustCompile(`https?://[^\s]+`) emailRegex = regexp.MustCompile(`[a-zA-Z0-9._%+\-]+@[a-zA-Z0-9.\-]+\.[a-zA-Z]{2,}`) - emojiRegex = regexp.MustCompile(`[\x{1F300}-\x{1F5FF}\x{1F600}-\x{1F64F}\x{1F680}-\x{1F6FF}\x{1F700}-\x{1F77F}\x{1F780}-\x{1F7FF}\x{1F800}-\x{1F8FF}\x{1F900}-\x{1F9FF}\x{1FA00}-\x{1FA6F}\x{1FA70}-\x{1FAFF}\x{1FB00}-\x{1FBFF}]+`) - applicantRoles = []string{"Founder or Owner", "Team Member or Employee", "Project Lead", "Core Maintainer", "Developer", "Organizer or Admin", "Program Manager"} + emojiRegex = regexp.MustCompile(`[\x{1F600}-\x{1F64F}\x{1F300}-\x{1F5FF}\x{1F680}-\x{1F6FF}\x{1F700}-\x{1F77F}\x{1F780}-\x{1F7FF}\x{1F800}-\x{1F8FF}\x{1F900}-\x{1F9FF}\x{1FA00}-\x{1FA6F}\x{1FA70}-\x{1FAFF}\x{1FB00}-\x{1FBFF}]+`) ) type ValidationError struct { @@ -54,8 +53,6 @@ func (v *Validator) HasError(section string) bool { return false } -// Parsing and validation utilities - func When(condition bool, callback ValidatorCallback) ValidatorCallback { if condition { return callback @@ -74,6 +71,30 @@ func ParseInput(value string) (bool, string, string) { return true, value, "" } +func ParsePlainString(value string) (bool, string, string) { + // strip all formattig, except for newlines + html := blackfriday.Run([]byte(value)) + doc, err := goquery.NewDocumentFromReader(bytes.NewReader(html)) + if err != nil { + return false, value, err.Error() + } + value = strings.TrimSpace(doc.Text()) + + if urlRegex.MatchString(value) { + return false, value, "cannot contain URLs" + } + + if emailRegex.MatchString(value) { + return false, value, "cannot contain email addresses" + } + + if emojiRegex.MatchString(value) { + return false, value, "cannot contain emoji characters" + } + + return true, value, "" +} + func ParseAccountUrl(value string) (bool, string, string) { if accountUrlRegex.Match([]byte(value)) { if !strings.HasPrefix(value, "http://") && !strings.HasPrefix(value, "https://") { @@ -87,7 +108,7 @@ func ParseAccountUrl(value string) (bool, string, string) { return true, u.Hostname(), "" } else { - return false, value, "is an invalid 1Password account URL" + return false, value, "is invalid 1Password account URL" } } @@ -168,40 +189,6 @@ func IsUrl(value string) (bool, string, string) { return true, value, "" } -func IsRegularString(value string) (bool, string, string) { - // strip all formattig, except for newlines - html := blackfriday.Run([]byte(value)) - doc, err := goquery.NewDocumentFromReader(bytes.NewReader(html)) - if err != nil { - return false, value, err.Error() - } - value = strings.TrimSpace(doc.Text()) - - if urlRegex.MatchString(value) { - return false, value, "cannot contain URLs" - } - - if emailRegex.MatchString(value) { - return false, value, "cannot contain email addresses" - } - - if emojiRegex.MatchString(value) { - return false, value, "cannot contain emoji characters" - } - - return true, value, "" -} - -func IsProjectRole(value string) (bool, string, string) { - for _, item := range applicantRoles { - if item == value { - return true, value, "" - } - } - - return false, value, "is an invalid project role" -} - func IsChecked(value string) (bool, string, string) { if value != "true" { return false, value, "must be checked" diff --git a/script/validator_test.go b/script/validator_test.go new file mode 100644 index 00000000..b6ac3090 --- /dev/null +++ b/script/validator_test.go @@ -0,0 +1,143 @@ +package main + +import ( + "testing" +) + +type validationTestCase struct { + input string + expectedPass bool + expectedValue string + expectedError string +} + +func runValidationTests(t *testing.T, testCases []validationTestCase, f ValidatorCallback, funcName string) { + for _, tc := range testCases { + pass, value, _ := f(tc.input) + if pass != tc.expectedPass { + t.Errorf("%s(%s) passing is %v, expected %v", funcName, tc.input, pass, tc.expectedPass) + } + + if value != tc.expectedValue { + t.Errorf("%s(%s) got new value %v, expected %v", funcName, tc.input, value, tc.expectedValue) + } + } +} + +func TestParseInput(t *testing.T) { + testCases := []validationTestCase{ + {"", true, "", ""}, + {"_No response_", true, "", ""}, + {"None", true, "", ""}, + {"hello", true, "hello", ""}, + } + runValidationTests(t, testCases, ParseInput, "ParseInput") +} + +func TestParsePlainString(t *testing.T) { + testCases := []validationTestCase{ + {"", true, "", ""}, + {"Hello world", true, "Hello world", ""}, + {"👋 howdy", false, "👋 howdy", "cannot contain emoji characters"}, + {"Testing formatting and link stripping", true, "Testing formatting and link stripping", ""}, + } + runValidationTests(t, testCases, ParsePlainString, "ParsePlainString") +} + +func TestParseAccountUrl(t *testing.T) { + testCases := []validationTestCase{ + {"myteam.1password.com", true, "myteam.1password.com", ""}, + } + runValidationTests(t, testCases, ParseAccountUrl, "ParseAccountUrl") +} + +func TestParseCheckbox(t *testing.T) { + testCases := []validationTestCase{ + {"[x]", true, "true", ""}, + {"[X]", true, "true", ""}, + {"[]", true, "false", ""}, + {"[ ]", true, "false", ""}, + {"- [x] Howdy", true, "true", ""}, + {"- [ ] Howdy", true, "false", ""}, + {"not a checkbox", false, "not a checkbox", "could not parse checkbox"}, + } + runValidationTests(t, testCases, ParseCheckbox, "ParseCheckbox") +} + +func TestParseNumber(t *testing.T) { + testCases := []struct { + input string + expectedPass bool + expectedValue int + expectedError string + }{ + {"123", true, 123, ""}, + {"abc123", true, 123, ""}, + {"123abc", true, 123, ""}, + {"abc", false, 0, "could not be parsed into a number"}, + } + + for _, tc := range testCases { + pass, value, _ := ParseNumber(tc.input) + if pass != tc.expectedPass { + t.Errorf("ParseNumber(%s) passing is %v, expected %v", tc.input, pass, tc.expectedPass) + } + + if value != tc.expectedValue { + t.Errorf("ParseNumber(%s) got new value %v, expected %v", tc.input, value, tc.expectedValue) + } + } +} + +func TestParseBool(t *testing.T) { + testCases := []struct { + input string + expectedPass bool + expectedValue bool + expectedError string + }{ + {"true", true, true, ""}, + {"false", true, false, ""}, + {"not a bool", false, false, "could not be parsed into a boolean"}, + } + + for _, tc := range testCases { + pass, value, _ := ParseBool(tc.input) + if pass != tc.expectedPass { + t.Errorf("ParseBool(%s) passing is %v, expected %v", tc.input, pass, tc.expectedPass) + } + + if value != tc.expectedValue { + t.Errorf("ParseBool(%s) got new value %v, expected %v", tc.input, value, tc.expectedValue) + } + } +} + +func TestIsPresent(t *testing.T) { + testCases := []validationTestCase{ + {"", false, "", "is empty"}, + {"foo", true, "foo", ""}, + } + runValidationTests(t, testCases, IsPresent, "IsPresent") +} + +func TestIsEmail(t *testing.T) { + testCases := []validationTestCase{ + {"", true, "", ""}, + } + runValidationTests(t, testCases, IsEmail, "IsEmail") +} + +func TestIsUrl(t *testing.T) { + testCases := []validationTestCase{ + {"", true, "", ""}, + } + runValidationTests(t, testCases, IsUrl, "IsUrl") +} + +func TestIsChecked(t *testing.T) { + testCases := []validationTestCase{ + {"", false, "", "must be checked"}, + } + runValidationTests(t, testCases, IsChecked, "IsChecked") +}