diff --git a/docs/images/arquillian_ui_failure_128px.png b/docs/images/arquillian_ui_failure_128px.png deleted file mode 100644 index 6b42ed4..0000000 Binary files a/docs/images/arquillian_ui_failure_128px.png and /dev/null differ diff --git a/docs/images/arquillian_ui_failure_64px.png b/docs/images/arquillian_ui_failure_64px.png new file mode 100644 index 0000000..2053511 Binary files /dev/null and b/docs/images/arquillian_ui_failure_64px.png differ diff --git a/docs/images/arquillian_ui_success_64px.png b/docs/images/arquillian_ui_success_64px.png new file mode 100644 index 0000000..0439f3d Binary files /dev/null and b/docs/images/arquillian_ui_success_64px.png differ diff --git a/glide.lock b/glide.lock index 4796d51..dd54493 100644 --- a/glide.lock +++ b/glide.lock @@ -1,5 +1,5 @@ hash: ec491e3f9011b7709d9ebc6cddbc6a0d8af5d72ac15e77aa01243f65499cf5f0 -updated: 2018-06-21T11:23:13.058723301+05:30 +updated: 2018-06-21T11:46:21.704131177+02:00 imports: - name: github.com/bazelbuild/buildtools version: 5a4c4ca9753ad0f8f9eb3463d84bc89388846420 @@ -15,9 +15,9 @@ imports: - name: github.com/evalphobia/logrus_sentry version: 57846a82817615f185b10cc40de8dc60c1ca5ae1 - name: github.com/getsentry/raven-go - version: ed7bcb39ff10f39ab08e317ce16df282845852fa + version: 563b81fc02b75d664e54da31f787c2cc2186780b - name: github.com/ghodss/yaml - version: e9ed3c6dfb39bb1a32197cb10d527906fe4da4b6 + version: 73d445a93680fa1a78ae23a5839bad48f32ba1ee - name: github.com/gogo/protobuf version: c0656edd0d9eab7c66d1eb0c568f9039345796f7 subpackages: @@ -28,7 +28,7 @@ imports: - name: github.com/golang/lint version: 470b6b0bb3005eda157f0275e2e4895055396a81 - name: github.com/golang/protobuf - version: 9f81198da99b79e14d70ca2c3cc1bbe44c6e69b6 + version: 1643683e1b54a9e88ad26d98f81400c8c9d9f4f9 subpackages: - proto - name: github.com/google/go-github @@ -48,7 +48,7 @@ imports: - name: github.com/gorilla/sessions version: 03b6f63cc43ef9c7240a635a5e22b13180e822b8 - name: github.com/matttproud/golang_protobuf_extensions - version: c12348ce28de40eed0136aa2b644d0ee0650e56c + version: fc2b8d3a73c4867e51861bbdd5ae3c1f0869dd6a subpackages: - pbutil - name: github.com/onsi/ginkgo @@ -94,20 +94,18 @@ imports: - prometheus - prometheus/promhttp - name: github.com/prometheus/client_model - version: 99fa1f4be8e564e8a6b613da7fa6f46c9edafc6c + version: fa8ad6fec33561be4280a8f0514318c79d7f6cb6 subpackages: - go - name: github.com/prometheus/common - version: 7600349dcfe1abd18d72d3a1770870d9800a7801 + version: 13ba4ddd0caa9c28ca7b7bffe1dfa9ed8d5ef207 subpackages: - expfmt - internal/bitbucket.org/ww/goautoneg - model - name: github.com/prometheus/procfs - version: 7d6f385de8bea29190f15ba9931442a0eaef9af7 + version: 65c1f6f8f0fc1e2185eb9863a3bc751496404259 subpackages: - - internal/util - - nfs - xfs - name: github.com/satori/go.uuid version: 879c5887cd475cd7864858769793b2ceb0d44feb @@ -127,14 +125,13 @@ imports: subpackages: - hooks/test - name: golang.org/x/crypto - version: 7f39a6fea4fe9364fb61e1def6a268a51b4f3a06 + version: 49796115aa4b964c318aad4f3084fdb41e9aa067 subpackages: - ssh/terminal - name: golang.org/x/net version: 1c05540f6879653db88113bc4a2b70aec4bd491f subpackages: - context - - context/ctxhttp - html - html/atom - html/charset @@ -143,7 +140,7 @@ imports: - idna - lex/httplex - name: golang.org/x/oauth2 - version: ef147856a6ddbb60760db74283d2424e98c87bff + version: a6bd8cefa1811bd24b86f8902872e4e8225f74c4 subpackages: - internal - name: golang.org/x/sys @@ -197,11 +194,11 @@ imports: - name: gopkg.in/yaml.v2 version: 5420a8b6744d3b0345ab293f6fcba19c978f1183 - name: k8s.io/api - version: 783dfbe86ff74ef4a6e1243688e1585ac243f8e7 + version: a464731143616c0241f473cebb050833c1f0b4fb subpackages: - core/v1 - name: k8s.io/apimachinery - version: 5a8013207d0d28c7fe98193e5b6cdbf92e98a000 + version: c3bfbaf8b18d67a795cc73475c511440d4b4de8d subpackages: - pkg/api/resource - pkg/apis/meta/v1 diff --git a/pkg/assets/config/message-with-hint-file-not-found.txt b/pkg/assets/config/message-with-hint-file-not-found.txt deleted file mode 100644 index 880492e..0000000 --- a/pkg/assets/config/message-with-hint-file-not-found.txt +++ /dev/null @@ -1,7 +0,0 @@ -{{.Thumbnail}} - -{{.Description}} - -Your plugin configuration is stored in the [file]({{.ConfigFile}}). - -In the configuration file you pointed to the custom comment file, but the plugin wasn't able to retrieve it from the defined location ({{.MessageFileURL}}). Make sure it is either a valid URL or a path to an existing file in this repository. diff --git a/pkg/config/config_loader.go b/pkg/config/config_loader.go index 2f168f5..4bb2729 100644 --- a/pkg/config/config_loader.go +++ b/pkg/config/config_loader.go @@ -16,7 +16,6 @@ type Source func() ([]byte, error) type PluginConfiguration struct { PluginName string LocationURL string - PluginHint string `yaml:"plugin_hint,omitempty"` } // Load loads configuration of the plugin based on strategies defined by SourcesProvider diff --git a/pkg/config/config_loader_test.go b/pkg/config/config_loader_test.go index 9921ef7..e65404c 100644 --- a/pkg/config/config_loader_test.go +++ b/pkg/config/config_loader_test.go @@ -24,9 +24,9 @@ var onlyName = config.Source(func() ([]byte, error) { return []byte("name: 'awesome-o'"), nil }) -var nameAndHint = config.Source(func() ([]byte, error) { - return []byte("name: 'name-and-hint'\n" + - "plugin_hint: 'I am just a hint'"), nil +var nameAndSkip = config.Source(func() ([]byte, error) { + return []byte("name: 'name-and-skip'\n" + + "skip_validation_for: ['anything']"), nil }) var faulty = config.Source(func() ([]byte, error) { @@ -93,7 +93,7 @@ var _ = Describe("Config loader features", func() { It("should load config from first working source (precedence)", func() { // given testConfigProviders := testConfigProvider(func() []config.Source { - return []config.Source{nameAndHint, onlyName} + return []config.Source{nameAndSkip, onlyName} }) sampleConfig := sampleConfiguration{Name: "prototype"} @@ -103,8 +103,8 @@ var _ = Describe("Config loader features", func() { // then Ω(err).ShouldNot(HaveOccurred()) - Expect(sampleConfig.Name).To(Equal("name-and-hint")) - Expect(sampleConfig.PluginHint).To(Equal("I am just a hint")) + Expect(sampleConfig.Name).To(Equal("name-and-skip")) + Expect(sampleConfig.Skip).To(ConsistOf("anything")) }) It("should preserve prototype config name when no sources provided", func() { diff --git a/pkg/github/client/client.go b/pkg/github/client/client.go index 68dbc07..abd465f 100644 --- a/pkg/github/client/client.go +++ b/pkg/github/client/client.go @@ -25,6 +25,7 @@ type Client interface { GetPullRequestReviews(owner, repo string, prNumber int) ([]*gogh.PullRequestReview, error) ListIssueComments(issue scm.RepositoryIssue) ([]*gogh.IssueComment, error) CreateIssueComment(issue scm.RepositoryIssue, commentMsg *string) error + EditIssueComment(issue scm.RepositoryIssue, commentID int64, commentMsg *string) error CreateStatus(change scm.RepositoryChange, repoStatus *gogh.RepoStatus) error AddPullRequestLabel(change scm.RepositoryChange, prNumber int, label []string) error RemovePullRequestLabel(change scm.RepositoryChange, prNumber int, label string) error @@ -160,6 +161,19 @@ func (c *client) CreateIssueComment(issue scm.RepositoryIssue, commentMsg *strin return err } +// EditIssueComment edits an already existing comment in the given issue. +func (c *client) EditIssueComment(issue scm.RepositoryIssue, commentID int64, commentMsg *string) error { + comment := &gogh.IssueComment{ + Body: commentMsg, + } + err := c.do(func(aroundContext aroundContext) (func(), *gogh.Response, error) { + _, response, e := c.gh.Issues.EditComment(context.Background(), issue.Owner, issue.RepoName, commentID, comment) + return func() {}, response, c.checkHTTPCode(response, e) + }) + + return err +} + // CreateStatus creates a new status for a repository at the specified reference represented by a RepositoryChange func (c *client) CreateStatus(change scm.RepositoryChange, repoStatus *gogh.RepoStatus) error { err := c.do(func(aroundContext aroundContext) (func(), *gogh.Response, error) { diff --git a/pkg/github/service/comment_service.go b/pkg/github/service/comment_service.go index ce28b97..fd87065 100644 --- a/pkg/github/service/comment_service.go +++ b/pkg/github/service/comment_service.go @@ -8,15 +8,15 @@ import ( // CommentService is a struct managing issue or pull request comments type CommentService struct { - client ghclient.Client - issue scm.RepositoryIssue + Client ghclient.Client + Issue scm.RepositoryIssue } // NewCommentService creates an instance of GitHub CommentService with information retrieved from the given IssueCommentEvents func NewCommentService(client ghclient.Client, comment *gogh.IssueCommentEvent) *CommentService { return &CommentService{ - client: client, - issue: scm.RepositoryIssue{ + Client: client, + Issue: scm.RepositoryIssue{ Owner: *comment.Repo.Owner.Login, RepoName: *comment.Repo.Name, Number: *comment.Issue.Number, @@ -26,5 +26,10 @@ func NewCommentService(client ghclient.Client, comment *gogh.IssueCommentEvent) // AddComment adds a comment message to the issue func (s *CommentService) AddComment(commentMsg *string) error { - return s.client.CreateIssueComment(s.issue, commentMsg) + return s.Client.CreateIssueComment(s.Issue, commentMsg) +} + +// EditComment edits an already existing comment message +func (s *CommentService) EditComment(commentID int64, commentMsg *string) error { + return s.Client.EditIssueComment(s.Issue, commentID, commentMsg) } diff --git a/pkg/github/service/hinter_service.go b/pkg/github/service/hinter_service.go deleted file mode 100644 index 302b19c..0000000 --- a/pkg/github/service/hinter_service.go +++ /dev/null @@ -1,78 +0,0 @@ -package ghservice - -import ( - "fmt" - "strings" - - "github.com/arquillian/ike-prow-plugins/pkg/github/client" - "github.com/arquillian/ike-prow-plugins/pkg/log" - "github.com/arquillian/ike-prow-plugins/pkg/utils" -) - -// PluginTitleTemplate is a constant template containing "Ike Plugins (name-of-plugin)" title with markdown formatting -const ( - PluginTitleTemplate = "### Ike Plugins (%s)" - assigneeMentionTemplate = "Thank you @%s for this contribution!" -) - -// Hinter is a struct managing plugin comments -type Hinter struct { - *CommentService - log log.Logger - commentContext HintContext - commentsLoader *IssueCommentsLazyLoader -} - -// HintContext holds a plugin name and a assignee to be mentioned in the comment -type HintContext struct { - PluginName string - Assignee string // TODO rethink this naming when plugins will start interacting with issue creators and reviewers -} - -// NewHinter creates an instance of GitHub Hinter for the given HintContext -func NewHinter(client ghclient.Client, log log.Logger, commentsLoader *IssueCommentsLazyLoader, commentContext HintContext) *Hinter { - return &Hinter{ - CommentService: &CommentService{ - client: client, - issue: commentsLoader.Issue, - }, - log: log, - commentContext: commentContext, - commentsLoader: commentsLoader, - } -} - -// PluginComment checks all present comments in the issue/pull-request. If no comment with PluginTitleTemplate -// (with the related plugin) is found, then it adds a new comment with the plugin title, assignee mention -// and the given commentMsg. If such a comment is present already, then it does nothing. -func (s *Hinter) PluginComment(commentMsg string) error { - comments, err := s.commentsLoader.Load() - if err != nil { - s.log.Errorf("Getting all comments failed with an error: %s", err) - } - - for _, com := range comments { - content := *com.Body - if strings.HasPrefix(content, s.getPluginTitle()) { - return nil - } - } - - return s.AddComment(s.createPluginHint(commentMsg)) -} - -func (s *Hinter) append(first, second string) string { - return first + "\n\n" + second -} - -func (s *Hinter) createPluginHint(commentMsg string) *string { - return utils.String(s.append(s.createBeginning(), commentMsg)) -} - -func (s *Hinter) createBeginning() string { - return s.append(s.getPluginTitle(), fmt.Sprintf(assigneeMentionTemplate, s.commentContext.Assignee)) -} - -func (s *Hinter) getPluginTitle() string { - return fmt.Sprintf(PluginTitleTemplate, s.commentContext.PluginName) -} diff --git a/pkg/hint/hint_message_loader.go b/pkg/hint/hint_message_loader.go deleted file mode 100644 index f29de2f..0000000 --- a/pkg/hint/hint_message_loader.go +++ /dev/null @@ -1,111 +0,0 @@ -package hint - -import ( - "bytes" - "fmt" - "net/url" - "regexp" - "text/template" - - assets "github.com/arquillian/ike-prow-plugins/pkg/assets/generated" - "github.com/arquillian/ike-prow-plugins/pkg/config" - "github.com/arquillian/ike-prow-plugins/pkg/github/service" - "github.com/arquillian/ike-prow-plugins/pkg/log" - "github.com/arquillian/ike-prow-plugins/pkg/scm" - "github.com/arquillian/ike-prow-plugins/pkg/utils" -) - -// Hint struct for the plugin hint service -type Hint struct { - Message *Message - Log log.Logger -} - -// Message struct for message template data for the plugin hint service -type Message struct { - Thumbnail string - Description string - ConfigFile string - Documentation string - MessageFileURL string -} - -// LoadMessage loads a hint message for the plugins from the template files. If the hint message is set in config then it takes that one, the default otherwise. -func (hint *Hint) LoadMessage(configuration config.PluginConfiguration, change scm.RepositoryChange) string { - var msg string - if configuration.PluginHint != "" { - msg = hint.getMsgFromConfigHint(configuration, change) - } else if content := hint.defaultFileContent(configuration, change); content != "" { - msg = content - } else if configuration.LocationURL == "" { - msg = hint.loadMessageTemplate("message-with-no-config.txt") - } else { - msg = hint.loadMessageTemplate("message-with-config.txt") - } - return hint.getMsgFromTemplate(msg) -} - -func (hint *Hint) getMsgFromTemplate(msg string) string { - var tpl bytes.Buffer - msgTemplate, err := template.New("message").Parse(msg) - if err != nil { - hint.Log.Errorf("falied to parse template file %s", err) - } - err = msgTemplate.Execute(&tpl, hint.Message) - if err != nil { - hint.Log.Errorf("failed to write template output %s", err) - } - return tpl.String() -} - -func (hint *Hint) getMsgFromConfigHint(configuration config.PluginConfiguration, change scm.RepositoryChange) string { - fileRegex := "(?mi)" + configuration.PluginName + "_hint.md$" - - isFilePath, err := regexp.MatchString(fileRegex, configuration.PluginHint) - if isFilePath && err == nil { - return hint.getMsgFromFile(configuration, change) - } - return configuration.PluginHint -} - -func (hint *Hint) getMsgFromFile(configuration config.PluginConfiguration, change scm.RepositoryChange) string { - _, err := url.ParseRequestURI(configuration.PluginHint) - - var content []byte - var msgFileURL string - - if err == nil { - msgFileURL = configuration.PluginHint - } else { - ghFileService := ghservice.RawFileService{Change: change} - msgFileURL = ghFileService.GetRawFileURL(configuration.PluginHint) - } - content, err = utils.GetFileFromURL(msgFileURL) - - if err != nil { - hint.Message.MessageFileURL = msgFileURL - return hint.loadMessageTemplate("message-with-hint-file-not-found.txt") - } - - return string(content) -} - -func (hint *Hint) defaultFileContent(configuration config.PluginConfiguration, change scm.RepositoryChange) string { - pluginHintPath := fmt.Sprintf("%s%s_hint.md", ghservice.ConfigHome, configuration.PluginName) - ghFileService := ghservice.RawFileService{Change: change} - hintURL := ghFileService.GetRawFileURL(pluginHintPath) - - content, e := utils.GetFileFromURL(hintURL) - if e != nil { - return "" - } - return string(content) -} - -func (hint *Hint) loadMessageTemplate(file string) string { - asset, err := assets.Asset(file) - if err != nil { - hint.Log.Errorf("failed to load template asset file %s", err) - } - return string(asset) -} diff --git a/pkg/hint/hint_message_loader_test.go b/pkg/hint/hint_message_loader_test.go deleted file mode 100644 index c012c74..0000000 --- a/pkg/hint/hint_message_loader_test.go +++ /dev/null @@ -1,351 +0,0 @@ -package hint_test - -import ( - "github.com/arquillian/ike-prow-plugins/pkg/config" - "github.com/arquillian/ike-prow-plugins/pkg/hint" - . "github.com/arquillian/ike-prow-plugins/pkg/internal/test" - "github.com/arquillian/ike-prow-plugins/pkg/scm" - . "github.com/onsi/ginkgo" - . "github.com/onsi/gomega" - "gopkg.in/h2non/gock.v1" -) - -const ProwPluginName string = "my-test-plugin" - -var _ = Describe("Hint message creation", func() { - - Context("Creation of default hint messages that are sent to a validated PR when custom message file is not set", func() { - - It("should create default message referencing to documentation when url to config is empty", func() { - // given - conf := config.PluginConfiguration{PluginName: ProwPluginName} - hint := &hint.Hint{ - Message: &hint.Message{ConfigFile: conf.LocationURL, Documentation: "#_my_test_plugin"}, - } - - // when - msg := hint.LoadMessage(conf, scm.RepositoryChange{}) - - // then - Expect(msg).To(ContainSubstring("http://arquillian.org/ike-prow-plugins/#_my_test_plugin")) - Expect(msg).NotTo(ContainSubstring("Your plugin configuration is stored in")) - }) - - It("should create default message referencing to config file when url to config is not empty", func() { - // given - url := "http://github.com/my/repo/my-test-plugin.yaml" - conf := config.PluginConfiguration{LocationURL: url} - hint := &hint.Hint{ - Message: &hint.Message{ConfigFile: conf.LocationURL, Documentation: "#_my_test_plugin"}, - } - - // when - msg := hint.LoadMessage(conf, scm.RepositoryChange{}) - - // then - Expect(msg).NotTo(ContainSubstring("http://arquillian.org/ike-prow-plugins/#_my_test_plugin")) - Expect(msg).To(ContainSubstring(url)) - }) - }) - - Context("Creation of default hint messages from default location when config plugin hint is not set", func() { - - BeforeEach(func() { - defer gock.OffAll() - }) - - AfterEach(EnsureGockRequestsHaveBeenMatched) - - It("should create message taken from a default hint file if plugin hint url is missing", func() { - // given - NonExistingRawGitHubFiles("my-test-plugin.yaml", "my-test-plugin.yml") - - gock.New("https://raw.githubusercontent.com"). - Get("owner/repo/46cb8fac44709e4ccaae97448c65e8f7320cfea7/.ike-prow/my-test-plugin_hint.md"). - Reply(200). - BodyString("Custom message") - - url := "http://github.com/my/repo/my-test-plugin.yaml" - conf := config.PluginConfiguration{ - LocationURL: url, - PluginName: ProwPluginName, - } - - change := scm.RepositoryChange{ - Owner: "owner", - RepoName: "repo", - Hash: "46cb8fac44709e4ccaae97448c65e8f7320cfea7", - } - - hint := &hint.Hint{ - Message: &hint.Message{ConfigFile: conf.LocationURL, Documentation: "#_my_test_plugin"}, - } - - // when - msg := hint.LoadMessage(conf, change) - - // then - Expect(msg).To(Equal("Custom message")) - }) - - It("should create message taken from a default hint file if location url is missing", func() { - // given - NonExistingRawGitHubFiles("my-test-plugin.yaml", "my-test-plugin.yml") - - gock.New("https://raw.githubusercontent.com"). - Get("owner/repo/46cb8fac44709e4ccaae97448c65e8f7320cfea7/.ike-prow/my-test-plugin_hint.md"). - Reply(200). - BodyString("Custom message") - - conf := config.PluginConfiguration{PluginName: ProwPluginName} - - change := scm.RepositoryChange{ - Owner: "owner", - RepoName: "repo", - Hash: "46cb8fac44709e4ccaae97448c65e8f7320cfea7", - } - - hint := &hint.Hint{ - Message: &hint.Message{ConfigFile: conf.LocationURL, Documentation: "#_my_test_plugin"}, - } - - // when - msg := hint.LoadMessage(conf, change) - - // then - Expect(msg).To(Equal("Custom message")) - }) - }) - - Context("Creation of default hint messages that are sent to a validated PR when custom message file is set", func() { - - BeforeEach(func() { - defer gock.OffAll() - }) - - AfterEach(EnsureGockRequestsHaveBeenMatched) - - It("should create message taken from a file set in config using relative path", func() { - // given - NonExistingRawGitHubFiles("my-test-plugin.yaml", "my-test-plugin.yml") - - gock.New("https://raw.githubusercontent.com"). - Get("owner/repo/46cb8fac44709e4ccaae97448c65e8f7320cfea7/path/to/my-test-plugin_hint.md"). - Reply(200). - BodyString("Custom message") - - url := "http://github.com/my/repo/my-test-plugin.yaml" - - conf := config.PluginConfiguration{ - LocationURL: url, - PluginName: ProwPluginName, - PluginHint: "path/to/my-test-plugin_hint.md", - } - - change := scm.RepositoryChange{ - Owner: "owner", - RepoName: "repo", - Hash: "46cb8fac44709e4ccaae97448c65e8f7320cfea7", - } - - hint := &hint.Hint{ - Message: &hint.Message{ConfigFile: conf.LocationURL, Documentation: "#_my_test_plugin"}, - } - - // when - msg := hint.LoadMessage(conf, change) - - // then - Expect(msg).To(Equal("Custom message")) - }) - - It("should create default message with no-found-custom-file suffix using wrong relative path", func() { - // given - NonExistingRawGitHubFiles("path/to/my-test-plugin_hint.md", "my-test-plugin.yaml", "my-test-plugin.yml") - - url := "http://github.com/my/repo/my-test-plugin.yaml" - conf := config.PluginConfiguration{ - LocationURL: url, - PluginName: ProwPluginName, - PluginHint: "path/to/my-test-plugin_hint.md", - } - - change := scm.RepositoryChange{ - Owner: "owner", - RepoName: "repo", - Hash: "46cb8fac44709e4ccaae97448c65e8f7320cfea7", - } - - hint := &hint.Hint{ - Message: &hint.Message{ConfigFile: conf.LocationURL, Documentation: "#_my_test_plugin"}, - } - - // when - msg := hint.LoadMessage(conf, change) - - // then - Expect(msg).NotTo(ContainSubstring("http://arquillian.org/ike-prow-plugins/#_my_test_plugin")) - Expect(msg).To(ContainSubstring(url)) - Expect(msg).To(ContainSubstring( - "https://raw.githubusercontent.com/owner/repo/46cb8fac44709e4ccaae97448c65e8f7320cfea7/" + - "path/to/my-test-plugin_hint.md")) - }) - - It("should create message taken from a file set in config using url", func() { - // given - gock.New("http://my.server.com"). - Get("path/to/my-test-plugin_hint.md").Reply(200). - BodyString("Custom message") - - url := "http://github.com/my/repo/my-test-plugin.yaml" - conf := config.PluginConfiguration{ - LocationURL: url, - PluginName: ProwPluginName, - PluginHint: "http://my.server.com/path/to/my-test-plugin_hint.md", - } - - hint := &hint.Hint{ - Message: &hint.Message{ConfigFile: conf.LocationURL, Documentation: "#_my_test_plugin"}, - } - - // when - msg := hint.LoadMessage(conf, scm.RepositoryChange{}) - - // then - Expect(msg).To(Equal("Custom message")) - }) - - It("should create message taken from a file with upper case filename set in config using url", func() { - // given - gock.New("http://my.server.com"). - Get("path/to/MY-TEST-PLUGIN_HINT.MD"). - Reply(200). - BodyString("Custom message") - - url := "http://github.com/my/repo/my-test-plugin.yaml" - conf := config.PluginConfiguration{ - LocationURL: url, - PluginName: ProwPluginName, - PluginHint: "http://my.server.com/path/to/MY-TEST-PLUGIN_HINT.MD", - } - - hint := &hint.Hint{ - Message: &hint.Message{ConfigFile: conf.LocationURL, Documentation: "#_my_test_plugin"}, - } - - // when - msg := hint.LoadMessage(conf, scm.RepositoryChange{}) - - // then - Expect(msg).To(Equal("Custom message")) - }) - - It("should create default message with no-found-custom-file suffix using wrong url path", func() { - // given - gock.New("http://my.server.com"). - Get("path/to/my-test-plugin_hint.md"). - Reply(404) - - url := "http://github.com/my/repo/my-test-plugin.yaml" - conf := config.PluginConfiguration{ - LocationURL: url, - PluginName: ProwPluginName, - PluginHint: "http://my.server.com/path/to/my-test-plugin_hint.md", - } - - hint := &hint.Hint{ - Message: &hint.Message{ConfigFile: conf.LocationURL, Documentation: "#_my_test_plugin"}, - } - - // when - msg := hint.LoadMessage(conf, scm.RepositoryChange{}) - - // then - Expect(msg).NotTo(ContainSubstring("http://arquillian.org/ike-prow-plugins/#_my_test_plugin")) - Expect(msg).To(ContainSubstring(url)) - Expect(msg).To(ContainSubstring( - "http://my.server.com/path/to/my-test-plugin_hint.md")) - }) - - It("should create default message with no-found-custom-file suffix using not-validate url", func() { - // given - NonExistingRawGitHubFiles("http/server.com/my-test-plugin_hint.md") - - gock.New("https://raw.githubusercontent.com"). - Get("owner/repo/46cb8fac44709e4ccaae97448c65e8f7320cfea7/path/to/my-test-plugin_hint.md"). - Reply(404) - - url := "http://github.com/my/repo/my-test-plugin.yaml" - conf := config.PluginConfiguration{ - LocationURL: url, - PluginName: ProwPluginName, - PluginHint: "http/server.com/my-test-plugin_hint.md", - } - - change := scm.RepositoryChange{ - Owner: "owner", - RepoName: "repo", - Hash: "46cb8fac44709e4ccaae97448c65e8f7320cfea7", - } - - hint := &hint.Hint{ - Message: &hint.Message{ConfigFile: conf.LocationURL, Documentation: "#_my_test_plugin"}, - } - - // when - msg := hint.LoadMessage(conf, change) - - // then - Expect(msg).NotTo(ContainSubstring("http://arquillian.org/ike-prow-plugins/#_my_test_plugin")) - Expect(msg).To(ContainSubstring(url)) - Expect(msg).To(ContainSubstring( - "https://raw.githubusercontent.com/owner/repo/46cb8fac44709e4ccaae97448c65e8f7320cfea7/" + - "http/server.com/my-test-plugin_hint.md")) - }) - - It("should create message taken from a string set in config", func() { - // given - url := "http://github.com/my/repo/my-test-plugin.yaml" - conf := config.PluginConfiguration{ - LocationURL: url, - PluginName: ProwPluginName, - PluginHint: "Custom message", - } - - hint := &hint.Hint{ - Message: &hint.Message{ConfigFile: conf.LocationURL, Documentation: "#_my_test_plugin"}, - } - - // when - msg := hint.LoadMessage(conf, scm.RepositoryChange{}) - - // then - Expect(msg).To(Equal("Custom message")) - }) - - It("should create message with file path as content using invalid filename pattern", func() { - // given - gock.New("http://my.server.com"). - Get("path/to/custom_message_file.md"). - Reply(404) - - url := "http://github.com/my/repo/my-test-plugin.yaml" - conf := config.PluginConfiguration{ - LocationURL: url, - PluginName: ProwPluginName, - PluginHint: "http://my.server.com/path/to/custom_message_file.md", - } - - hint := &hint.Hint{ - Message: &hint.Message{ConfigFile: conf.LocationURL, Documentation: "#_my_test_plugin"}, - } - - // when - msg := hint.LoadMessage(conf, scm.RepositoryChange{}) - - // then - Expect(msg).NotTo(ContainSubstring("http://arquillian.org/ike-prow-plugins/#_my_test_plugin")) - Expect(msg).To(ContainSubstring("http://my.server.com/path/to/custom_message_file.md")) - }) - }) -}) diff --git a/pkg/internal/test/test_doubles.go b/pkg/internal/test/test_doubles.go index f29978c..1a44bd2 100644 --- a/pkg/internal/test/test_doubles.go +++ b/pkg/internal/test/test_doubles.go @@ -8,6 +8,7 @@ import ( "github.com/arquillian/ike-prow-plugins/pkg/github/client" "github.com/arquillian/ike-prow-plugins/pkg/log" + "github.com/arquillian/ike-prow-plugins/pkg/utils" gogh "github.com/google/go-github/github" "github.com/onsi/ginkgo" ) @@ -32,6 +33,26 @@ func FromFile(filePath string) io.Reader { return file } +// NewPullRequest creates an instance of gogh.PullRequest with the given values +func NewPullRequest(repoOwner, repoName, sha, creator string) *gogh.PullRequest { + return &gogh.PullRequest{ + Base: &gogh.PullRequestBranch{ + Repo: &gogh.Repository{ + Name: utils.String(repoName), + Owner: &gogh.User{ + Login: utils.String(repoOwner), + }, + }, + }, + Head: &gogh.PullRequestBranch{ + SHA: utils.String(sha), + }, + User: &gogh.User{ + Login: utils.String(creator), + }, + } +} + // NewDefaultGitHubClient creates a GH client with default go-github client (without any authentication token) func NewDefaultGitHubClient() ghclient.Client { client := ghclient.NewClient(gogh.NewClient(nil), log.NewTestLogger()) diff --git a/pkg/plugin/pr-sanitizer/event_handler.go b/pkg/plugin/pr-sanitizer/event_handler.go index e7ebc3b..831f64d 100644 --- a/pkg/plugin/pr-sanitizer/event_handler.go +++ b/pkg/plugin/pr-sanitizer/event_handler.go @@ -6,10 +6,10 @@ import ( "github.com/arquillian/ike-prow-plugins/pkg/github" "github.com/arquillian/ike-prow-plugins/pkg/github/client" - "github.com/arquillian/ike-prow-plugins/pkg/github/service" "github.com/arquillian/ike-prow-plugins/pkg/log" "github.com/arquillian/ike-prow-plugins/pkg/plugin/work-in-progress" "github.com/arquillian/ike-prow-plugins/pkg/scm" + "github.com/arquillian/ike-prow-plugins/pkg/status" "github.com/arquillian/ike-prow-plugins/pkg/utils" gogh "github.com/google/go-github/github" ) @@ -63,7 +63,7 @@ func (gh *GitHubLabelsEventsHandler) HandleEvent(log log.Logger, eventType githu Hash: *event.PullRequest.Head.SHA, } statusContext := github.StatusContext{BotName: gh.BotName, PluginName: ProwPluginName} - statusService := ghservice.NewStatusService(gh.Client, log, change, statusContext) + statusService := status.NewStatusService(gh.Client, log, change, statusContext) config := LoadConfiguration(log, change) if gh.HasTitleWithValidType(config, *event.PullRequest.Title) { diff --git a/pkg/plugin/test-keeper/configuration_gh_test.go b/pkg/plugin/test-keeper/configuration_gh_test.go index 963028d..6c33eb8 100644 --- a/pkg/plugin/test-keeper/configuration_gh_test.go +++ b/pkg/plugin/test-keeper/configuration_gh_test.go @@ -30,8 +30,7 @@ var _ = Describe("Test keeper config loader features", func() { Get("owner/repo/46cb8fac44709e4ccaae97448c65e8f7320cfea7/" + configFilePath + ".yml"). Reply(200). BodyString("test_patterns: ['*my', 'test.go', 'pattern.js']\n" + - "skip_validation_for: ['pom.xml', 'regex{{*\\.adoc}}']\n" + - "plugin_hint: 'http://my.server.com/message.md'") + "skip_validation_for: ['pom.xml', 'regex{{*\\.adoc}}']\n") change := scm.RepositoryChange{ Owner: "owner", @@ -45,7 +44,6 @@ var _ = Describe("Test keeper config loader features", func() { // then Expect(configuration.LocationURL).To(Equal("https://github.com/owner/repo/blob/46cb8fac44709e4ccaae97448c65e8f7320cfea7/.ike-prow/test-keeper.yml")) Expect(configuration.PluginName).To(Equal(testkeeper.ProwPluginName)) - Expect(configuration.PluginHint).To(Equal("http://my.server.com/message.md")) }) It("should load test-keeper configuration yml file", func() { @@ -54,8 +52,7 @@ var _ = Describe("Test keeper config loader features", func() { Get("owner/repo/46cb8fac44709e4ccaae97448c65e8f7320cfea7/" + configFilePath + ".yml"). Reply(200). BodyString("test_patterns: ['*my', 'test.go', 'pattern.js']\n" + - "skip_validation_for: ['pom.xml', 'regex{{*\\.adoc}}']\n" + - "plugin_hint: 'http://my.server.com/message.md'") + "skip_validation_for: ['pom.xml', 'regex{{*\\.adoc}}']\n") change := scm.RepositoryChange{ Owner: "owner", @@ -67,7 +64,6 @@ var _ = Describe("Test keeper config loader features", func() { configuration := testkeeper.LoadConfiguration(logger, change) // then - Expect(configuration.PluginHint).To(Equal("http://my.server.com/message.md")) Expect(configuration.Inclusions).To(ConsistOf("*my", "test.go", "pattern.js")) Expect(configuration.Exclusions).To(ConsistOf("pom.xml", "regex{{*\\.adoc}}")) Expect(configuration.Combine).To(BeTrue()) @@ -81,8 +77,7 @@ var _ = Describe("Test keeper config loader features", func() { Get("owner/repo/46cb8fac44709e4ccaae97448c65e8f7320cfea7/" + configFilePath + ".yaml"). Reply(200). BodyString("test_patterns: ['*my', 'test.go', 'pattern.js']\n" + - "skip_validation_for: ['pom.xml', 'regex{{*\\.adoc}}']\n" + - "plugin_hint: 'http://my.server.com/message.md'") + "skip_validation_for: ['pom.xml', 'regex{{*\\.adoc}}']\n") change := scm.RepositoryChange{ Owner: "owner", @@ -94,7 +89,6 @@ var _ = Describe("Test keeper config loader features", func() { configuration := testkeeper.LoadConfiguration(logger, change) // then - Expect(configuration.PluginHint).To(Equal("http://my.server.com/message.md")) Expect(configuration.Inclusions).To(ConsistOf("*my", "test.go", "pattern.js")) Expect(configuration.Exclusions).To(ConsistOf("pom.xml", "regex{{*\\.adoc}}")) Expect(configuration.Combine).To(BeTrue()) @@ -115,7 +109,6 @@ var _ = Describe("Test keeper config loader features", func() { // then Expect(configuration.LocationURL).To(BeEmpty()) - Expect(configuration.PluginHint).To(BeEmpty()) Expect(configuration.Inclusions).To(BeEmpty()) Expect(configuration.Exclusions).To(BeEmpty()) Expect(configuration.Combine).To(BeTrue()) diff --git a/pkg/plugin/test-keeper/event_handler.go b/pkg/plugin/test-keeper/event_handler.go index 65ca6ff..e4dcc0b 100644 --- a/pkg/plugin/test-keeper/event_handler.go +++ b/pkg/plugin/test-keeper/event_handler.go @@ -113,8 +113,8 @@ func (gh *GitHubTestEventsHandler) handlePrComment(log log.Logger, comment *gogh if err != nil { return err } - statusService := gh.newTestStatusService(log, ghservice.NewRepositoryChangeForPR(pullRequest)) reportBypassCommand(pullRequest) + statusService := gh.newTestStatusService(log, pullRequest) return statusService.okWithoutTests(*comment.Sender.Login) }}) @@ -133,7 +133,9 @@ func (gh *GitHubTestEventsHandler) checkTestsAndSetStatus(log log.Logger, prLoad change := ghservice.NewRepositoryChangeForPR(pr) configuration := LoadConfiguration(log, change) fileCategories, err := gh.checkTests(log, change, configuration, *pr.Number) - statusService := gh.newTestStatusService(log, change) + commentsLoader := ghservice.NewIssueCommentsLazyLoader(gh.Client, pr) + + statusService := gh.newTestStatusServiceWithMessages(log, pr, commentsLoader, configuration) if err != nil { if statusErr := statusService.reportError(); statusErr != nil { log.Errorf("failed to report error status on PR [%q]. cause: %s", *pr, statusErr) @@ -142,15 +144,16 @@ func (gh *GitHubTestEventsHandler) checkTestsAndSetStatus(log log.Logger, prLoad } if fileCategories.OnlySkippedFiles() { + statusService.onlySkippedMessage() return statusService.okOnlySkippedFiles() } if fileCategories.TestsExist() { reportPullRequest(log, pr, WithTests) + statusService.withTestsMessage() return statusService.okTestsExist() } - commentsLoader := ghservice.NewIssueCommentsLazyLoader(gh.Client, pr) bypassed, user := gh.checkIfBypassed(log, commentsLoader, pr) if bypassed { reportBypassCommand(pr) @@ -158,20 +161,11 @@ func (gh *GitHubTestEventsHandler) checkTestsAndSetStatus(log log.Logger, prLoad } reportPullRequest(log, pr, WithoutTests) + statusService.withoutTestsMessage() err = statusService.failNoTests() if err != nil { log.Errorf("failed to report status on PR [%q]. cause: %s", *pr, err) } - - hintContext := ghservice.HintContext{PluginName: ProwPluginName, Assignee: *pr.User.Login} - hinter := ghservice.NewHinter(gh.Client, log, commentsLoader, hintContext) - - cerr := hinter.PluginComment(CreateHintMessage(configuration, change, log)) - if cerr != nil { - log.Errorf("failed to comment on PR [%q]. cause: %s", *pr, cerr) - return cerr - } - return err } diff --git a/pkg/plugin/test-keeper/event_handler_gh_test.go b/pkg/plugin/test-keeper/event_handler_gh_test.go index 9ef8973..d66bdb5 100644 --- a/pkg/plugin/test-keeper/event_handler_gh_test.go +++ b/pkg/plugin/test-keeper/event_handler_gh_test.go @@ -11,6 +11,7 @@ import ( "github.com/arquillian/ike-prow-plugins/pkg/log" "github.com/arquillian/ike-prow-plugins/pkg/plugin" "github.com/arquillian/ike-prow-plugins/pkg/plugin/test-keeper" + "github.com/arquillian/ike-prow-plugins/pkg/status/message" . "github.com/onsi/ginkgo" . "github.com/onsi/gomega" "gopkg.in/h2non/gock.v1" @@ -44,8 +45,9 @@ var _ = Describe("Test Keeper Plugin features", func() { } toHaveBodyWithWholePluginsComment := SoftlySatisfyAll( - HaveBodyThatContains(fmt.Sprintf(ghservice.PluginTitleTemplate, testkeeper.ProwPluginName)), + HaveBodyThatContains(fmt.Sprintf(message.PluginTitleTemplate, testkeeper.ProwPluginName)), HaveBodyThatContains("@bartoszmajsak"), + HaveBodyThatContains("It appears that no tests have been added or updated in this PR."), ) Context("Pull Request event handling", func() { @@ -57,16 +59,29 @@ var _ = Describe("Test Keeper Plugin features", func() { AfterEach(EnsureGockRequestsHaveBeenMatched) - It("should approve opened pull request when tests included", func() { + It("should approve opened pull request and update status message when tests included", func() { // given - NonExistingRawGitHubFiles("test-keeper.yml", "test-keeper.yaml", "test-keeper_hint.md") - gockEmptyComments(2) + NonExistingRawGitHubFiles("test-keeper.yml", "test-keeper.yaml", "test-keeper_with_tests_message.md") gock.New("https://api.github.com"). Get("/repos/" + repositoryName + "/pulls/2/files"). Reply(200). Body(FromFile("test_fixtures/github_calls/prs/with_tests/changes.json")) + gock.New("https://api.github.com"). + Get("/repos/" + repositoryName + "/issues/2/comments"). + Reply(200). + Body(FromFile("test_fixtures/github_calls/prs/comments_with_no_test_status_msg.json")) + + gock.New("https://api.github.com"). + Patch("/repos/" + repositoryName + "/issues/comments/397622617"). + SetMatcher(ExpectPayload(SoftlySatisfyAll( + HaveBodyThatContains(fmt.Sprintf(message.PluginTitleTemplate, testkeeper.ProwPluginName)), + HaveBodyThatContains("@bartoszmajsak"), + HaveBodyThatContains("It seems that this PR already contains some added or changed tests. Good job!"), + ))). + Reply(201) + gock.New("https://api.github.com"). Post("/repos/" + repositoryName + "/statuses"). SetMatcher(ExpectPayload(toBe(github.StatusSuccess, testkeeper.TestsExistMessage, expectedContext, testkeeper.TestsExistDetailsPageName))). @@ -140,8 +155,7 @@ var _ = Describe("Test Keeper Plugin features", func() { It("should reject opened pull request when no tests are matching defined pattern with no defaults implicitly combined", func() { // given - - NonExistingRawGitHubFiles("test-keeper_hint.md") + NonExistingRawGitHubFiles("test-keeper_without_tests_message.md") gockEmptyComments(2) gock.New("https://raw.githubusercontent.com"). @@ -177,7 +191,7 @@ var _ = Describe("Test Keeper Plugin features", func() { It("should block newly created pull request when no tests are included", func() { // given - NonExistingRawGitHubFiles("test-keeper.yml", "test-keeper.yaml", "test-keeper_hint.md") + NonExistingRawGitHubFiles("test-keeper.yml", "test-keeper.yaml", "test-keeper_without_tests_message.md") gockEmptyComments(1) gock.New("https://api.github.com"). @@ -206,7 +220,7 @@ var _ = Describe("Test Keeper Plugin features", func() { It("should not block newly created pull request when documentation and build files are the only changes", func() { // given - NonExistingRawGitHubFiles("test-keeper.yml", "test-keeper.yaml") + NonExistingRawGitHubFiles("test-keeper.yml", "test-keeper.yaml", "test-keeper_only_skipped_message.md") gockEmptyComments(1) gock.New("https://api.github.com"). @@ -230,7 +244,7 @@ var _ = Describe("Test Keeper Plugin features", func() { It("should block newly created pull request when deletions in the tests are the only changes", func() { // given - NonExistingRawGitHubFiles("test-keeper.yml", "test-keeper.yaml", "test-keeper_hint.md") + NonExistingRawGitHubFiles("test-keeper.yml", "test-keeper.yaml", "test-keeper_without_tests_message.md") gockEmptyComments(1) gock.New("https://api.github.com"). @@ -259,7 +273,7 @@ var _ = Describe("Test Keeper Plugin features", func() { It("should block newly created pull request when there are changes in the business logic but only deletions in the tests", func() { // given - NonExistingRawGitHubFiles("test-keeper.yml", "test-keeper.yaml", "test-keeper_hint.md") + NonExistingRawGitHubFiles("test-keeper.yml", "test-keeper.yaml", "test-keeper_without_tests_message.md") gockEmptyComments(1) gock.New("https://api.github.com"). @@ -330,13 +344,12 @@ var _ = Describe("Test Keeper Plugin features", func() { It("should block pull request without tests and with comments containing bypass message added by user with insufficient permissions", func() { // given - NonExistingRawGitHubFiles("test-keeper.yml", "test-keeper.yaml", "test-keeper_hint.md") + NonExistingRawGitHubFiles("test-keeper.yml", "test-keeper.yaml", "test-keeper_without_tests_message.md") gock.New("https://api.github.com"). Get("/repos/" + repositoryName + "/issues/1/comments"). Reply(200). - BodyString(`[{"user":{"login":"bartoszmajsak-test"}, "body":"` + testkeeper.BypassCheckComment + `"},` + - `{"body":"` + fmt.Sprintf(ghservice.PluginTitleTemplate, testkeeper.ProwPluginName) + `"}]`) + Body(FromFile("test_fixtures/github_calls/prs/comments_with_no_test_status_msg.json")) gock.New("https://api.github.com"). Get("/repos/" + repositoryName + "/pulls/1/reviews"). @@ -466,7 +479,7 @@ var _ = Describe("Test Keeper Plugin features", func() { AfterEach(EnsureGockRequestsHaveBeenMatched) It("should block newly created pull request without tests when "+command.RunCommentPrefix+" all command is used by admin user", func() { - NonExistingRawGitHubFiles("test-keeper.yml", "test-keeper.yaml", "test-keeper_hint.md") + NonExistingRawGitHubFiles("test-keeper.yml", "test-keeper.yaml", "test-keeper_without_tests_message.md") gock.New("https://api.github.com"). Get("/repos/" + repositoryName + "/pulls/1"). @@ -513,7 +526,7 @@ var _ = Describe("Test Keeper Plugin features", func() { }) It("should approve newly created pull request with tests when "+command.RunCommentPrefix+" "+testkeeper.ProwPluginName+" command is triggered by pr reviewer", func() { - NonExistingRawGitHubFiles("test-keeper.yml", "test-keeper.yaml") + NonExistingRawGitHubFiles("test-keeper.yml", "test-keeper.yaml", "test-keeper_with_tests_message.md") gock.New("https://api.github.com"). Get("/repos/" + repositoryName + "/pulls/2"). diff --git a/pkg/plugin/test-keeper/hint_message.go b/pkg/plugin/test-keeper/hint_message.go deleted file mode 100644 index b3e7baf..0000000 --- a/pkg/plugin/test-keeper/hint_message.go +++ /dev/null @@ -1,36 +0,0 @@ -package testkeeper - -import ( - "github.com/arquillian/ike-prow-plugins/pkg/hint" - "github.com/arquillian/ike-prow-plugins/pkg/log" - "github.com/arquillian/ike-prow-plugins/pkg/scm" -) - -const ( - paragraph = "\n\n" - - beginning = "It appears that no tests have been added or updated in this PR." + - paragraph + - "Automated tests give us confidence in shipping reliable software. Please add some as part of this change." + - paragraph + - "If you are an admin or the reviewer of this PR and you are sure that no test is needed then you can use the command `" + BypassCheckComment + "` " + - "as a comment to make the status green.\n" - - documentationSection = "#_test_keeper_plugin" - - sadIke = `` -) - -// CreateHintMessage creates a hint message for the test-keeper plugin. If the hint message is set in config then it takes that one, the default otherwise. -func CreateHintMessage(configuration PluginConfiguration, change scm.RepositoryChange, log log.Logger) string { - pluginHint := &hint.Hint{ - Log: log, - Message: &hint.Message{ - Thumbnail: sadIke, - Description: beginning, - ConfigFile: configuration.LocationURL, - Documentation: documentationSection, - }, - } - return pluginHint.LoadMessage(configuration.PluginConfiguration, change) -} diff --git a/pkg/plugin/test-keeper/metrics_test.go b/pkg/plugin/test-keeper/metrics_test.go index cbc8940..f965b1c 100644 --- a/pkg/plugin/test-keeper/metrics_test.go +++ b/pkg/plugin/test-keeper/metrics_test.go @@ -6,10 +6,10 @@ import ( "strings" "github.com/arquillian/ike-prow-plugins/pkg/github" - "github.com/arquillian/ike-prow-plugins/pkg/github/service" . "github.com/arquillian/ike-prow-plugins/pkg/internal/test" "github.com/arquillian/ike-prow-plugins/pkg/log" "github.com/arquillian/ike-prow-plugins/pkg/plugin/test-keeper" + "github.com/arquillian/ike-prow-plugins/pkg/status/message" "github.com/arquillian/ike-prow-plugins/pkg/utils" . "github.com/onsi/ginkgo" . "github.com/onsi/gomega" @@ -32,7 +32,7 @@ var _ = Describe("TestKeeper Metrics", func() { } toHaveBodyWithWholePluginsComment := SoftlySatisfyAll( - HaveBodyThatContains(fmt.Sprintf(ghservice.PluginTitleTemplate, testkeeper.ProwPluginName)), + HaveBodyThatContains(fmt.Sprintf(message.PluginTitleTemplate, testkeeper.ProwPluginName)), HaveBodyThatContains("@bartoszmajsak"), ) @@ -140,7 +140,7 @@ var _ = Describe("TestKeeper Metrics", func() { It("should report pull requests without tests", func() { //given - NonExistingRawGitHubFiles("test-keeper.yml", "test-keeper.yaml", "test-keeper_hint.md") + NonExistingRawGitHubFiles("test-keeper.yml", "test-keeper.yaml", "test-keeper_without_tests_message.md") gockEmptyComments(1) gock.New("https://api.github.com"). diff --git a/pkg/plugin/test-keeper/test_fixtures/github_calls/prs/comments_with_no_test_status_msg.json b/pkg/plugin/test-keeper/test_fixtures/github_calls/prs/comments_with_no_test_status_msg.json new file mode 100644 index 0000000..0686bac --- /dev/null +++ b/pkg/plugin/test-keeper/test_fixtures/github_calls/prs/comments_with_no_test_status_msg.json @@ -0,0 +1,64 @@ +[ + { + "url": "https://api.github.com/repos/bartoszmajsak/wfswarm-booster-pipeline-test/issues/comments/397622617", + "html_url": "https://github.com/bartoszmajsak/wfswarm-booster-pipeline-test/pull/1#issuecomment-397622617", + "issue_url": "https://api.github.com/repos/bartoszmajsak/wfswarm-booster-pipeline-test/issues/1", + "id": 397622617, + "node_id": "MDEyOklzc3VlQ29tbWVudDM5NzYyMjYxNw==", + "user": { + "login": "bartoszmajsak", + "id": 1510709, + "node_id": "MDQ6VXNlcjE1MTA3MDk=", + "avatar_url": "https://avatars3.githubusercontent.com/u/1510709?v=4", + "gravatar_id": "", + "url": "https://api.github.com/users/bartoszmajsak", + "html_url": "https://github.com/bartoszmajsak", + "followers_url": "https://api.github.com/users/bartoszmajsak/followers", + "following_url": "https://api.github.com/users/bartoszmajsak/following{/other_user}", + "gists_url": "https://api.github.com/users/bartoszmajsak/gists{/gist_id}", + "starred_url": "https://api.github.com/users/bartoszmajsak/starred{/owner}{/repo}", + "subscriptions_url": "https://api.github.com/users/bartoszmajsak/subscriptions", + "organizations_url": "https://api.github.com/users/bartoszmajsak/orgs", + "repos_url": "https://api.github.com/users/bartoszmajsak/repos", + "events_url": "https://api.github.com/users/bartoszmajsak/events{/privacy}", + "received_events_url": "https://api.github.com/users/bartoszmajsak/received_events", + "type": "User", + "site_admin": false + }, + "created_at": "2018-06-15T13:37:36Z", + "updated_at": "2018-06-15T13:53:44Z", + "author_association": "OWNER", + "body": "### Ike Plugins (test-keeper)\n\nThank you @bartoszmajsak-test for this contribution!\n\n\n\nIt appears that no tests have been added or updated in this PR.\n\nAutomated tests give us confidence in shipping reliable software. Please add some as part of this change.\n\nIf you are an admin or the reviewer of this PR and you are sure that no test is needed then you can use the command `/ok-without-tests` as a comment to make the status green.\n\n\nFor more information please head over to official [documentation](http://arquillian.org/ike-prow-plugins/#_test_keeper_plugin). You can find there how to configure the plugin.\n" + }, + { + "url": "https://api.github.com/repos/bartoszmajsak/wfswarm-booster-pipeline-test/issues/comments/397624053", + "html_url": "https://github.com/bartoszmajsak/wfswarm-booster-pipeline-test/pull/1#issuecomment-397624053", + "issue_url": "https://api.github.com/repos/bartoszmajsak/wfswarm-booster-pipeline-test/issues/1", + "id": 397624053, + "node_id": "MDEyOklzc3VlQ29tbWVudDM5NzYyNDA1Mw==", + "user": { + "login": "bartoszmajsak-test", + "id": 1510709, + "node_id": "MDQ6VXNlcjE1MTA3MDk=", + "avatar_url": "https://avatars3.githubusercontent.com/u/1510709?v=4", + "gravatar_id": "", + "url": "https://api.github.com/users/bartoszmajsak-test", + "html_url": "https://github.com/bartoszmajsak-test", + "followers_url": "https://api.github.com/users/bartoszmajsak-test/followers", + "following_url": "https://api.github.com/users/bartoszmajsak-test/following{/other_user}", + "gists_url": "https://api.github.com/users/bartoszmajsak-test/gists{/gist_id}", + "starred_url": "https://api.github.com/users/bartoszmajsak-test/starred{/owner}{/repo}", + "subscriptions_url": "https://api.github.com/users/bartoszmajsak-test/subscriptions", + "organizations_url": "https://api.github.com/users/bartoszmajsak-test/orgs", + "repos_url": "https://api.github.com/users/bartoszmajsak-test/repos", + "events_url": "https://api.github.com/users/bartoszmajsak-test/events{/privacy}", + "received_events_url": "https://api.github.com/users/bartoszmajsak-test/received_events", + "type": "User", + "site_admin": false + }, + "created_at": "2018-06-15T13:43:03Z", + "updated_at": "2018-06-18T11:29:52Z", + "author_association": "OWNER", + "body": "/ok-without-tests" + } +] \ No newline at end of file diff --git a/pkg/plugin/test-keeper/test_fixtures/github_calls/prs/with_tests/test-keeper.yml b/pkg/plugin/test-keeper/test_fixtures/github_calls/prs/with_tests/test-keeper.yml index f391b1a..77c9558 100644 --- a/pkg/plugin/test-keeper/test_fixtures/github_calls/prs/with_tests/test-keeper.yml +++ b/pkg/plugin/test-keeper/test_fixtures/github_calls/prs/with_tests/test-keeper.yml @@ -2,5 +2,4 @@ test_patterns: - '**/__test.go' # skip_validation_for: - 'regex{{.*\.[xsd]?html$}}' # -combine_defaults: false # -plugin_hint: .ike-prow/test-keeper_hint.md # \ No newline at end of file +combine_defaults: false # \ No newline at end of file diff --git a/pkg/plugin/test-keeper/test_keeper_status_service.go b/pkg/plugin/test-keeper/test_keeper_status_service.go new file mode 100644 index 0000000..e93945d --- /dev/null +++ b/pkg/plugin/test-keeper/test_keeper_status_service.go @@ -0,0 +1,131 @@ +package testkeeper + +import ( + "fmt" + + "github.com/arquillian/ike-prow-plugins/pkg/github" + "github.com/arquillian/ike-prow-plugins/pkg/github/service" + "github.com/arquillian/ike-prow-plugins/pkg/log" + "github.com/arquillian/ike-prow-plugins/pkg/scm" + "github.com/arquillian/ike-prow-plugins/pkg/status" + "github.com/arquillian/ike-prow-plugins/pkg/status/message" + gogh "github.com/google/go-github/github" +) + +type testStatusService struct { + log log.Logger + change scm.RepositoryChange + statusService scm.StatusService +} + +const ( + // TestsExistMessage is a message used in GH Status as description when tests are found + TestsExistMessage = "There are some tests :)" + // TestsExistDetailsPageName is a name of a documentation page that contains additional status details for TestsExistMessage + TestsExistDetailsPageName = "tests-exist" + + // NoTestsMessage is a message used in GH Status as description when no tests shipped with the PR + NoTestsMessage = "No tests in this PR :(" + // NoTestsDetailsPageName is a name of a documentation page that contains additional status details for NoTestsMessage + NoTestsDetailsPageName = "no-tests" + + // OkOnlySkippedFilesMessage is a message used in GH Status as description when PR comes with a changeset which shouldn't be subject of test verification + OkOnlySkippedFilesMessage = "Seems that this PR doesn't need to have tests" + // OkOnlySkippedFilesDetailsPageName is a name of a documentation page that contains additional status details for OkOnlySkippedFilesMessage + OkOnlySkippedFilesDetailsPageName = "only-skipped" + + // FailureMessage is a message used in GH Status as description when failure occurred + FailureMessage = "Failed while check for tests" + + // ApprovedByMessage is a message used in GH Status as description when it's commented to skip the check + ApprovedByMessage = "PR is fine without tests says @%s" + // ApprovedByDetailsPageName is a name of a documentation page that contains additional status details for ApprovedByMessage + ApprovedByDetailsPageName = "keeper-approved-by" +) + +func (gh *GitHubTestEventsHandler) newTestStatusService(log log.Logger, pullRequest *gogh.PullRequest) *testStatusService { + change := ghservice.NewRepositoryChangeForPR(pullRequest) + statusContext := github.StatusContext{BotName: gh.BotName, PluginName: ProwPluginName} + statusService := status.NewStatusService(gh.Client, log, change, statusContext) + return &testStatusService{ + log: log, + change: change, + statusService: statusService, + } +} + +func (ts *testStatusService) okTestsExist() error { + return ts.statusService.Success(TestsExistMessage, TestsExistDetailsPageName) +} + +func (ts *testStatusService) okOnlySkippedFiles() error { + return ts.statusService.Success(OkOnlySkippedFilesMessage, OkOnlySkippedFilesDetailsPageName) +} + +func (ts *testStatusService) okWithoutTests(approvedBy string) error { + return ts.statusService.Success(fmt.Sprintf(ApprovedByMessage, approvedBy), ApprovedByDetailsPageName) +} + +func (ts *testStatusService) reportError() error { + return ts.statusService.Error(FailureMessage) +} + +func (ts *testStatusService) failNoTests() error { + return ts.statusService.Failure(NoTestsMessage, NoTestsDetailsPageName) +} + +const ( + paragraph = "\n\n" + + withoutTestsMsg = "It appears that no tests have been added or updated in this PR." + + paragraph + + "Automated tests give us confidence in shipping reliable software. Please add some as part of this change." + + paragraph + + "If you are an admin or the reviewer of this PR and you are sure that no test is needed then you can use the command `" + BypassCheckComment + "` " + + "as a comment to make the status green.\n" + + documentationSection = "#_test_keeper_plugin" + + withTestsMsg = "It seems that this PR already contains some added or changed tests. Good job!" + + onlySkippedMsg = "It seems that this PR doesn't need any test as all changed files in the changeset match " + + "patterns for which the validation should be skipped." +) + +type testStatusServiceWithMessages struct { + *testStatusService + statusMsgService *message.StatusMessageService + config PluginConfiguration +} + +func (gh *GitHubTestEventsHandler) newTestStatusServiceWithMessages(log log.Logger, pullRequest *gogh.PullRequest, + commentsLoader *ghservice.IssueCommentsLazyLoader, config PluginConfiguration) testStatusServiceWithMessages { + + msgContext := message.NewStatusMessageContext(ProwPluginName, documentationSection, pullRequest, config.PluginConfiguration) + msgService := message.NewStatusMessageService(gh.Client, log, commentsLoader, msgContext) + + return testStatusServiceWithMessages{ + testStatusService: gh.newTestStatusService(log, pullRequest), + statusMsgService: msgService, + config: config, + } +} + +// CreateWithoutTestsMessage creates a status message for the test-keeper plugin. If the status message is set in config then it takes that one, the default otherwise. +func (ts *testStatusServiceWithMessages) withoutTestsMessage() { + ts.logError(ts.statusMsgService.SadStatusMessage(withoutTestsMsg, "without_tests", true)) +} + +// CreateWithTestsMessage creates a status message for the test-keeper plugin. If the status message is set in config then it takes that one, the default otherwise. +func (ts *testStatusServiceWithMessages) withTestsMessage() { + ts.logError(ts.statusMsgService.HappyStatusMessage(withTestsMsg, "with_tests", false)) +} + +// CreateOnlySkippedMessage creates a status message for the test-keeper plugin. If the status message is set in config then it takes that one, the default otherwise. +func (ts *testStatusServiceWithMessages) onlySkippedMessage() { + ts.logError(ts.statusMsgService.HappyStatusMessage(onlySkippedMsg, "only_skipped", false)) +} + +func (ts *testStatusServiceWithMessages) logError(err error) { + ts.log.Errorf("failed to comment on PR, caused by: %s", err) +} diff --git a/pkg/plugin/test-keeper/test_status_service.go b/pkg/plugin/test-keeper/test_status_service.go deleted file mode 100644 index fcf45f8..0000000 --- a/pkg/plugin/test-keeper/test_status_service.go +++ /dev/null @@ -1,65 +0,0 @@ -package testkeeper - -import ( - "fmt" - - "github.com/arquillian/ike-prow-plugins/pkg/github" - "github.com/arquillian/ike-prow-plugins/pkg/github/service" - "github.com/arquillian/ike-prow-plugins/pkg/log" - "github.com/arquillian/ike-prow-plugins/pkg/scm" -) - -type testStatusService struct { - statusService scm.StatusService -} - -const ( - // TestsExistMessage is a message used in GH Status as description when tests are found - TestsExistMessage = "There are some tests :)" - // TestsExistDetailsPageName is a name of a documentation page that contains additional status details for TestsExistMessage - TestsExistDetailsPageName = "tests-exist" - - // NoTestsMessage is a message used in GH Status as description when no tests shipped with the PR - NoTestsMessage = "No tests in this PR :(" - // NoTestsDetailsPageName is a name of a documentation page that contains additional status details for NoTestsMessage - NoTestsDetailsPageName = "no-tests" - - // OkOnlySkippedFilesMessage is a message used in GH Status as description when PR comes with a changeset which shouldn't be subject of test verification - OkOnlySkippedFilesMessage = "Seems that this PR doesn't need to have tests" - // OkOnlySkippedFilesDetailsPageName is a name of a documentation page that contains additional status details for OkOnlySkippedFilesMessage - OkOnlySkippedFilesDetailsPageName = "only-skipped" - - // FailureMessage is a message used in GH Status as description when failure occurred - FailureMessage = "Failed while check for tests" - - // ApprovedByMessage is a message used in GH Status as description when it's commented to skip the check - ApprovedByMessage = "PR is fine without tests says @%s" - // ApprovedByDetailsPageName is a name of a documentation page that contains additional status details for ApprovedByMessage - ApprovedByDetailsPageName = "keeper-approved-by" -) - -func (gh *GitHubTestEventsHandler) newTestStatusService(log log.Logger, change scm.RepositoryChange) testStatusService { - statusContext := github.StatusContext{BotName: gh.BotName, PluginName: ProwPluginName} - statusService := ghservice.NewStatusService(gh.Client, log, change, statusContext) - return testStatusService{statusService: statusService} -} - -func (ts *testStatusService) okTestsExist() error { - return ts.statusService.Success(TestsExistMessage, TestsExistDetailsPageName) -} - -func (ts *testStatusService) okOnlySkippedFiles() error { - return ts.statusService.Success(OkOnlySkippedFilesMessage, OkOnlySkippedFilesDetailsPageName) -} - -func (ts *testStatusService) okWithoutTests(approvedBy string) error { - return ts.statusService.Success(fmt.Sprintf(ApprovedByMessage, approvedBy), ApprovedByDetailsPageName) -} - -func (ts *testStatusService) reportError() error { - return ts.statusService.Error(FailureMessage) -} - -func (ts *testStatusService) failNoTests() error { - return ts.statusService.Failure(NoTestsMessage, NoTestsDetailsPageName) -} diff --git a/pkg/plugin/work-in-progress/event_handler.go b/pkg/plugin/work-in-progress/event_handler.go index 8587855..aceb74a 100644 --- a/pkg/plugin/work-in-progress/event_handler.go +++ b/pkg/plugin/work-in-progress/event_handler.go @@ -11,6 +11,7 @@ import ( "github.com/arquillian/ike-prow-plugins/pkg/github/client" "github.com/arquillian/ike-prow-plugins/pkg/github/service" "github.com/arquillian/ike-prow-plugins/pkg/log" + "github.com/arquillian/ike-prow-plugins/pkg/status" "github.com/arquillian/ike-prow-plugins/pkg/utils" gogh "github.com/google/go-github/github" ) @@ -122,7 +123,7 @@ func (gh *GitHubWIPPRHandler) handlePrComment(log log.Logger, comment *gogh.Issu func (gh *GitHubWIPPRHandler) checkComponentsAndSetStatus(log log.Logger, pullRequest *gogh.PullRequest, labelUpdated bool) error { change := ghservice.NewRepositoryChangeForPR(pullRequest) statusContext := github.StatusContext{BotName: gh.BotName, PluginName: ProwPluginName} - statusService := ghservice.NewStatusService(gh.Client, log, change, statusContext) + statusService := status.NewStatusService(gh.Client, log, change, statusContext) configuration := LoadConfiguration(log, change) labelExists := gh.hasWorkInProgressLabel(pullRequest.Labels, configuration.Label) diff --git a/pkg/status/message/message_loader.go b/pkg/status/message/message_loader.go new file mode 100644 index 0000000..4da52a5 --- /dev/null +++ b/pkg/status/message/message_loader.go @@ -0,0 +1,78 @@ +package message + +import ( + "bytes" + "fmt" + "text/template" + + "github.com/arquillian/ike-prow-plugins/pkg/assets/generated" + "github.com/arquillian/ike-prow-plugins/pkg/github/service" + "github.com/arquillian/ike-prow-plugins/pkg/log" + "github.com/arquillian/ike-prow-plugins/pkg/scm" + "github.com/arquillian/ike-prow-plugins/pkg/utils" +) + +// Loader keeps information necessary for status message loading +type Loader struct { + Message *Message + Log log.Logger + PluginName string +} + +// Message keeps all data used in message templates +type Message struct { + Thumbnail string + Description string + ConfigFile string + Documentation string + MessageFileURL string +} + +// LoadMessage loads a status message from the template files +func (l *Loader) LoadMessage(change scm.RepositoryChange, statusFileSpec string) string { + var msg string + + if content := l.defaultFileContent(l.PluginName, change, statusFileSpec); content != "" { + msg = content + } else if l.Message.ConfigFile == "" { + msg = l.loadMessageTemplate("message-with-no-config.txt") + } else { + msg = l.loadMessageTemplate("message-with-config.txt") + } + return l.getMsgFromTemplate(msg) +} + +func (l *Loader) getMsgFromTemplate(msg string) string { + var tpl bytes.Buffer + msgTemplate, err := template.New("message").Parse(msg) + if err != nil { + l.Log.Errorf("failed to parse template file %s", err) + } + err = msgTemplate.Execute(&tpl, l.Message) + if err != nil { + l.Log.Errorf("failed to write template output %s", err) + } + return tpl.String() +} + +func (l *Loader) defaultFileContent(pluginName string, change scm.RepositoryChange, defaultFileSpec string) string { + if defaultFileSpec != "" { + defaultFileSpec = "_" + defaultFileSpec + } + statusMsgPath := fmt.Sprintf("%s%s%s_message.md", ghservice.ConfigHome, pluginName, defaultFileSpec) + ghFileService := ghservice.RawFileService{Change: change} + + content, e := utils.GetFileFromURL(ghFileService.GetRawFileURL(statusMsgPath)) + if e != nil { + return "" + } + return string(content) +} + +func (l *Loader) loadMessageTemplate(file string) string { + asset, err := assets.Asset(file) + if err != nil { + l.Log.Errorf("failed to load template asset file %s", err) + } + return string(asset) +} diff --git a/pkg/status/message/message_loader_test.go b/pkg/status/message/message_loader_test.go new file mode 100644 index 0000000..954b0ea --- /dev/null +++ b/pkg/status/message/message_loader_test.go @@ -0,0 +1,115 @@ +package message_test + +import ( + "github.com/arquillian/ike-prow-plugins/pkg/config" + . "github.com/arquillian/ike-prow-plugins/pkg/internal/test" + "github.com/arquillian/ike-prow-plugins/pkg/scm" + "github.com/arquillian/ike-prow-plugins/pkg/status/message" + . "github.com/onsi/ginkgo" + . "github.com/onsi/gomega" + "gopkg.in/h2non/gock.v1" +) + +const ProwPluginName string = "my-test-plugin" + +var _ = Describe("Loader message creation", func() { + + Context("Creation of default message messages that are sent to a validated PR when custom message file is not set", func() { + + It("should create default message referencing to documentation when url to config is empty", func() { + // given + conf := config.PluginConfiguration{PluginName: ProwPluginName} + message := &message.Loader{ + Message: &message.Message{ConfigFile: conf.LocationURL, Documentation: "#_my_test_plugin"}, + } + + // when + msg := message.LoadMessage(scm.RepositoryChange{}, "") + + // then + Expect(msg).To(ContainSubstring("http://arquillian.org/ike-prow-plugins/#_my_test_plugin")) + Expect(msg).NotTo(ContainSubstring("Your plugin configuration is stored in")) + }) + + It("should create default message referencing to config file when url to config is not empty", func() { + // given + url := "http://github.com/my/repo/my-test-plugin.yaml" + conf := config.PluginConfiguration{LocationURL: url} + message := &message.Loader{ + Message: &message.Message{ConfigFile: conf.LocationURL, Documentation: "#_my_test_plugin"}, + } + + // when + msg := message.LoadMessage(scm.RepositoryChange{}, "") + + // then + Expect(msg).NotTo(ContainSubstring("http://arquillian.org/ike-prow-plugins/#_my_test_plugin")) + Expect(msg).To(ContainSubstring(url)) + }) + }) + + Context("Creation of default message messages from default location when config plugin message is not set", func() { + + BeforeEach(func() { + defer gock.OffAll() + }) + + AfterEach(EnsureGockRequestsHaveBeenMatched) + + It("should create message taken from a default message file if plugin message url is missing", func() { + // given + NonExistingRawGitHubFiles("my-test-plugin.yaml", "my-test-plugin.yml") + + gock.New("https://raw.githubusercontent.com"). + Get("owner/repo/46cb8fac44709e4ccaae97448c65e8f7320cfea7/.ike-prow/my-test-plugin_message.md"). + Reply(200). + BodyString("Custom message") + + change := scm.RepositoryChange{ + Owner: "owner", + RepoName: "repo", + Hash: "46cb8fac44709e4ccaae97448c65e8f7320cfea7", + } + + message := &message.Loader{ + PluginName: ProwPluginName, + Message: &message.Message{ + ConfigFile: "http://github.com/my/repo/my-test-plugin.yaml", + Documentation: "#_my_test_plugin"}, + } + + // when + msg := message.LoadMessage(change, "") + + // then + Expect(msg).To(Equal("Custom message")) + }) + + It("should create message taken from a default message file if location url is missing", func() { + // given + NonExistingRawGitHubFiles("my-test-plugin.yaml", "my-test-plugin.yml") + + gock.New("https://raw.githubusercontent.com"). + Get("owner/repo/46cb8fac44709e4ccaae97448c65e8f7320cfea7/.ike-prow/my-test-plugin_message.md"). + Reply(200). + BodyString("Custom message") + + change := scm.RepositoryChange{ + Owner: "owner", + RepoName: "repo", + Hash: "46cb8fac44709e4ccaae97448c65e8f7320cfea7", + } + + message := &message.Loader{ + PluginName: ProwPluginName, + Message: &message.Message{Documentation: "#_my_test_plugin"}, + } + + // when + msg := message.LoadMessage(change, "") + + // then + Expect(msg).To(Equal("Custom message")) + }) + }) +}) diff --git a/pkg/status/message/message_service.go b/pkg/status/message/message_service.go new file mode 100644 index 0000000..8669a9e --- /dev/null +++ b/pkg/status/message/message_service.go @@ -0,0 +1,142 @@ +package message + +import ( + "fmt" + "strings" + + "github.com/arquillian/ike-prow-plugins/pkg/config" + "github.com/arquillian/ike-prow-plugins/pkg/github/client" + "github.com/arquillian/ike-prow-plugins/pkg/github/service" + "github.com/arquillian/ike-prow-plugins/pkg/log" + "github.com/arquillian/ike-prow-plugins/pkg/scm" + "github.com/arquillian/ike-prow-plugins/pkg/utils" + gogh "github.com/google/go-github/github" +) + +// PluginTitleTemplate is a constant template containing "Ike Plugins (name-of-plugin)" title with markdown formatting +const ( + PluginTitleTemplate = "### Ike Plugins (%s)" + assigneeMentionTemplate = "Thank you @%s for this contribution!" + + sadIke = `` + happyIke = `` +) + +// StatusMessageService is a struct managing plugin comments +type StatusMessageService struct { + commentService *ghservice.CommentService + log log.Logger + commentContext StatusMessageContext + commentsLoader *ghservice.IssueCommentsLazyLoader + change scm.RepositoryChange +} + +// StatusMessageContext holds a plugin name and a assignee to be mentioned in the comment +type StatusMessageContext struct { + pluginName string + documentationSection string + pullRequest *gogh.PullRequest + config config.PluginConfiguration +} + +// NewStatusMessageContext creates an instance of StatusMessageContext with the given values +func NewStatusMessageContext(pluginName, documentationSection string, pr *gogh.PullRequest, config config.PluginConfiguration) StatusMessageContext { + return StatusMessageContext{ + pluginName: pluginName, + documentationSection: documentationSection, + pullRequest: pr, + config: config, + } +} + +// NewStatusMessageService creates an instance of GitHub StatusMessageService for the given StatusMessageContext +func NewStatusMessageService(client ghclient.Client, log log.Logger, commentsLoader *ghservice.IssueCommentsLazyLoader, commentContext StatusMessageContext) *StatusMessageService { + return &StatusMessageService{ + commentService: &ghservice.CommentService{ + Client: client, + Issue: commentsLoader.Issue, + }, + log: log, + commentContext: commentContext, + commentsLoader: commentsLoader, + change: ghservice.NewRepositoryChangeForPR(commentContext.pullRequest), + } +} + +// SadStatusMessage creates a message with the sad Ike image +func (s *StatusMessageService) SadStatusMessage(description, statusFileSpec string, addIfMissing bool) error { + return s.StatusMessage(func() string { + messageLoader := s.newMessageLoader(sadIke, description) + return messageLoader.LoadMessage(s.change, statusFileSpec) + }, addIfMissing) +} + +// HappyStatusMessage creates a message with the happy Ike image +func (s *StatusMessageService) HappyStatusMessage(description, statusFileSpec string, addIfMissing bool) error { + return s.StatusMessage(func() string { + messageLoader := s.newMessageLoader(happyIke, description) + return messageLoader.LoadMessage(s.change, statusFileSpec) + }, addIfMissing) +} + +func (s *StatusMessageService) newMessageLoader(image, msg string) *Loader { + return &Loader{ + Log: s.log, + PluginName: s.commentContext.pluginName, + Message: &Message{ + Thumbnail: image, + Description: msg, + ConfigFile: s.commentContext.config.LocationURL, + Documentation: s.commentContext.documentationSection, + }, + } +} + +// StatusMessage checks all present comments in the issue/pull-request. If no comment with PluginTitleTemplate +// (with the related plugin) is found, then it adds a new comment with the plugin title, assignee mention +// and the given commentMsg. If such a comment is present already, then it does nothing. +func (s *StatusMessageService) StatusMessage(commentMsgCreator func() string, addIfMissing bool) error { + comments, err := s.commentsLoader.Load() + if err != nil { + s.log.Errorf("Getting all comments failed with an error: %s", err) + } + statusMsg := utils.String("") + + for _, com := range comments { + content := *com.Body + if strings.HasPrefix(content, s.getPluginTitle()) { + s.loadStatusMessage(commentMsgCreator, statusMsg) + if strings.TrimSpace(content) == strings.TrimSpace(*statusMsg) { + return nil + } + return s.commentService.EditComment(*com.ID, statusMsg) + } + } + if addIfMissing { + s.loadStatusMessage(commentMsgCreator, statusMsg) + return s.commentService.AddComment(statusMsg) + } + return nil +} + +func (s *StatusMessageService) append(first, second string) string { + return first + "\n\n" + second +} + +func (s *StatusMessageService) createPluginStatusMsg(commentMsg string) *string { + return utils.String(s.append(s.createBeginning(), commentMsg)) +} + +func (s *StatusMessageService) createBeginning() string { + return s.append(s.getPluginTitle(), fmt.Sprintf(assigneeMentionTemplate, *s.commentContext.pullRequest.User.Login)) +} + +func (s *StatusMessageService) getPluginTitle() string { + return fmt.Sprintf(PluginTitleTemplate, s.commentContext.pluginName) +} + +func (s *StatusMessageService) loadStatusMessage(commentMsgCreator func() string, statusMsg *string) { + if *statusMsg == "" { + *statusMsg = *s.createPluginStatusMsg(commentMsgCreator()) + } +} diff --git a/pkg/github/service/hinter_service_test.go b/pkg/status/message/message_service_test.go similarity index 51% rename from pkg/github/service/hinter_service_test.go rename to pkg/status/message/message_service_test.go index aac5dbd..72d5a45 100644 --- a/pkg/github/service/hinter_service_test.go +++ b/pkg/status/message/message_service_test.go @@ -1,11 +1,13 @@ -package ghservice_test +package message_test import ( + "github.com/arquillian/ike-prow-plugins/pkg/config" "github.com/arquillian/ike-prow-plugins/pkg/github/client" "github.com/arquillian/ike-prow-plugins/pkg/github/service" . "github.com/arquillian/ike-prow-plugins/pkg/internal/test" "github.com/arquillian/ike-prow-plugins/pkg/log" "github.com/arquillian/ike-prow-plugins/pkg/scm" + "github.com/arquillian/ike-prow-plugins/pkg/status/message" . "github.com/onsi/ginkgo" . "github.com/onsi/gomega" "gopkg.in/h2non/gock.v1" @@ -36,11 +38,9 @@ var _ = Describe("Config loader features", func() { Client: client, Issue: *scm.NewRepositoryIssue("owner", "repo", 2), } - hintContext := ghservice.HintContext{ - PluginName: "my-plugin-name", - Assignee: "toAssign", - } - hinter := ghservice.NewHinter(client, log.NewTestLogger(), commentsLoader, hintContext) + messageContext := message.NewStatusMessageContext("my-plugin-name", "docSection", + NewPullRequest("owner", "repo", "1a2b", "toAssign"), config.PluginConfiguration{}) + msgService := message.NewStatusMessageService(client, log.NewTestLogger(), commentsLoader, messageContext) toHaveBodyWithWholePluginsComment := SoftlySatisfyAll( HaveBodyThatContains("### Ike Plugins (my-plugin-name)"), @@ -54,7 +54,7 @@ var _ = Describe("Config loader features", func() { Reply(201) // when - err := hinter.PluginComment("New comment") + err := msgService.StatusMessage(newBasicMsgCreator("New comment"), true) // then Ω(err).ShouldNot(HaveOccurred()) @@ -71,21 +71,19 @@ var _ = Describe("Config loader features", func() { Client: client, Issue: *scm.NewRepositoryIssue("owner", "repo", 2), } - hintContext := ghservice.HintContext{ - PluginName: "test-keeper", - Assignee: "toAssign", - } + messageContext := message.NewStatusMessageContext("test-keeper", "docSection", + NewPullRequest("owner", "repo", "1a2b", "toAssign"), config.PluginConfiguration{}) - hinter := ghservice.NewHinter(client, log.NewTestLogger(), commentsLoader, hintContext) + msgService := message.NewStatusMessageService(client, log.NewTestLogger(), commentsLoader, messageContext) // when - err := hinter.PluginComment("New comment") + err := msgService.StatusMessage(newBasicMsgCreator("Message from test-keeper"), true) // then Ω(err).ShouldNot(HaveOccurred()) }) - It("should create a new comment that contains missing plugin hint when different one already exists", func() { + It("should create a new comment that contains missing status message when different one already exists", func() { // given gock.New("https://api.github.com"). Get("/repos/owner/repo/issues/2/comments"). @@ -96,10 +94,8 @@ var _ = Describe("Config loader features", func() { Client: client, Issue: *scm.NewRepositoryIssue("owner", "repo", 2), } - hintContext := ghservice.HintContext{ - PluginName: "another-plugin", - Assignee: "toAssign", - } + messageContext := message.NewStatusMessageContext("another-plugin", "docSection", + NewPullRequest("owner", "repo", "1a2b", "toAssign"), config.PluginConfiguration{}) expContent := "### Ike Plugins (another-plugin)\n\nThank you @toAssign for this contribution!" + "\n\nNew comment" @@ -113,13 +109,53 @@ var _ = Describe("Config loader features", func() { SetMatcher(ExpectPayload(toHaveModifiedBody)). Reply(200) - hinter := ghservice.NewHinter(client, log.NewTestLogger(), commentsLoader, hintContext) + msgService := message.NewStatusMessageService(client, log.NewTestLogger(), commentsLoader, messageContext) + + // when + err := msgService.StatusMessage(newBasicMsgCreator("New comment"), true) + + // then + Ω(err).ShouldNot(HaveOccurred()) + }) + + It("should modify existing comment when a new message is provided", func() { + // given + gock.New("https://api.github.com"). + Get("/repos/owner/repo/issues/2/comments"). + Reply(200). + Body(FromFile("test_fixtures/github_calls/prs/comments_with_tests_keeper_message.json")) + + commentsLoader := &ghservice.IssueCommentsLazyLoader{ + Client: client, + Issue: *scm.NewRepositoryIssue("owner", "repo", 2), + } + messageContext := message.NewStatusMessageContext("test-keeper", "docSection", + NewPullRequest("owner", "repo", "1a2b", "toAssign"), config.PluginConfiguration{}) + + toHaveBodyWithWholePluginsComment := SoftlySatisfyAll( + HaveBodyThatContains("### Ike Plugins (test-keeper)"), + HaveBodyThatContains("@toAssign"), + HaveBodyThatContains("New Message"), + ) + + gock.New("https://api.github.com"). + Patch("/repos/owner/repo/issues/comments/372707978"). + SetMatcher(ExpectPayload(toHaveBodyWithWholePluginsComment)). + Reply(201) + + msgService := message.NewStatusMessageService(client, log.NewTestLogger(), commentsLoader, messageContext) // when - err := hinter.PluginComment("New comment") + err := msgService.StatusMessage(newBasicMsgCreator("New Message"), true) // then Ω(err).ShouldNot(HaveOccurred()) }) }) }) + +func newBasicMsgCreator(msg string) func() string { + return func() string { + return msg + } +} diff --git a/pkg/hint/hint_suite_test.go b/pkg/status/message/message_suite_test.go similarity index 72% rename from pkg/hint/hint_suite_test.go rename to pkg/status/message/message_suite_test.go index 1b2ea03..7ee1f8f 100644 --- a/pkg/hint/hint_suite_test.go +++ b/pkg/status/message/message_suite_test.go @@ -1,4 +1,4 @@ -package hint_test +package message import ( "testing" @@ -10,5 +10,5 @@ import ( func TestConfig(t *testing.T) { RegisterFailHandler(Fail) - RunSpecWithJUnitReporter(t, "Plugin Comment Loader Suite") + RunSpecWithJUnitReporter(t, "Status Message Loader Suite") } diff --git a/pkg/github/service/test_fixtures/github_calls/prs/comments_with_tests_keeper_message.json b/pkg/status/message/test_fixtures/github_calls/prs/comments_with_tests_keeper_message.json similarity index 95% rename from pkg/github/service/test_fixtures/github_calls/prs/comments_with_tests_keeper_message.json rename to pkg/status/message/test_fixtures/github_calls/prs/comments_with_tests_keeper_message.json index c200ede..5201966 100644 --- a/pkg/github/service/test_fixtures/github_calls/prs/comments_with_tests_keeper_message.json +++ b/pkg/status/message/test_fixtures/github_calls/prs/comments_with_tests_keeper_message.json @@ -26,7 +26,7 @@ "created_at": "2018-03-13T15:33:32Z", "updated_at": "2018-03-13T15:33:32Z", "author_association": "OWNER", - "body": "### Ike Plugins (test-keeper)\n\n@MatousJobanek Please pay attention to the following message:\n\nMessage from test-keeper" + "body": "### Ike Plugins (test-keeper)\n\nThank you @toAssign for this contribution!\n\nMessage from test-keeper" }, { "url": "https://api.github.com/repos/MatousJobanek/testing-repo/issues/comments/372945558", diff --git a/pkg/github/service/status_service.go b/pkg/status/status_service.go similarity index 77% rename from pkg/github/service/status_service.go rename to pkg/status/status_service.go index 16a367d..74515cf 100644 --- a/pkg/github/service/status_service.go +++ b/pkg/status/status_service.go @@ -1,4 +1,4 @@ -package ghservice +package status import ( "fmt" @@ -14,17 +14,17 @@ import ( "github.com/google/go-github/github" ) -// StatusService is a struct -type StatusService struct { +// Service is a struct containing information necessary for status setting +type Service struct { client ghclient.Client log log.Logger statusContext github_type.StatusContext change scm.RepositoryChange } -// NewStatusService creates an instance of GitHub StatusService +// NewStatusService creates an instance of Service necessary for setting status func NewStatusService(client ghclient.Client, log log.Logger, change scm.RepositoryChange, context github_type.StatusContext) scm.StatusService { - return &StatusService{ + return &Service{ client: client, log: log, statusContext: context, @@ -33,27 +33,27 @@ func NewStatusService(client ghclient.Client, log log.Logger, change scm.Reposit } // Success marks given change as a success. -func (s *StatusService) Success(reason, detailsPageName string) error { +func (s *Service) Success(reason, detailsPageName string) error { return s.setStatus(github_type.StatusSuccess, reason, s.generateDetailsLink(detailsPageName, github_type.StatusSuccess)) } // Failure marks given change as a failure. -func (s *StatusService) Failure(reason, detailsPageName string) error { +func (s *Service) Failure(reason, detailsPageName string) error { return s.setStatus(github_type.StatusFailure, reason, s.generateDetailsLink(detailsPageName, github_type.StatusFailure)) } // Pending marks given change as a pending. -func (s *StatusService) Pending(reason string) error { +func (s *Service) Pending(reason string) error { return s.setStatus(github_type.StatusPending, reason, "") } // Error marks given change as a error. -func (s *StatusService) Error(reason string) error { +func (s *Service) Error(reason string) error { return s.setStatus(github_type.StatusError, reason, "") } // setStatus sets the given status with the given reason to the related commit -func (s *StatusService) setStatus(status, reason, detailsLink string) error { +func (s *Service) setStatus(status, reason, detailsLink string) error { c := fmt.Sprintf("%s/%s", s.statusContext.BotName, s.statusContext.PluginName) repoStatus := github.RepoStatus{ State: &status, @@ -71,6 +71,6 @@ func (s *StatusService) setStatus(status, reason, detailsLink string) error { return err } -func (s StatusService) generateDetailsLink(filename, status string) string { +func (s Service) generateDetailsLink(filename, status string) string { return fmt.Sprintf("%s/status/%s/%s/%s.html", plugin.DocumentationURL, s.statusContext.PluginName, strings.ToLower(status), filename) } diff --git a/pkg/github/service/status_service_test.go b/pkg/status/status_service_test.go similarity index 93% rename from pkg/github/service/status_service_test.go rename to pkg/status/status_service_test.go index 14ab961..3146018 100644 --- a/pkg/github/service/status_service_test.go +++ b/pkg/status/status_service_test.go @@ -1,12 +1,12 @@ -package ghservice_test +package status_test import ( "github.com/arquillian/ike-prow-plugins/pkg/github" - "github.com/arquillian/ike-prow-plugins/pkg/github/service" . "github.com/arquillian/ike-prow-plugins/pkg/internal/test" "github.com/arquillian/ike-prow-plugins/pkg/log" "github.com/arquillian/ike-prow-plugins/pkg/plugin" "github.com/arquillian/ike-prow-plugins/pkg/scm" + "github.com/arquillian/ike-prow-plugins/pkg/status" . "github.com/onsi/ginkgo" . "github.com/onsi/gomega" "gopkg.in/h2non/gock.v1" @@ -31,7 +31,7 @@ var _ = Describe("GitHub Status Service", func() { defer gock.OffAll() change := scm.RepositoryChange{RepoName: "test-repo", Owner: "alien-ike", Hash: "1232asdasd"} context := github.StatusContext{BotName: "alien-ike", PluginName: "test-keeper"} - statusService = ghservice.NewStatusService(NewDefaultGitHubClient(), log.NewTestLogger(), change, context) + statusService = status.NewStatusService(NewDefaultGitHubClient(), log.NewTestLogger(), change, context) }) AfterEach(EnsureGockRequestsHaveBeenMatched)