Skip to content
This repository was archived by the owner on Sep 30, 2024. It is now read-only.

gqltest: add failure message when Bitbucket Permissions job failed to run#39428

Merged
sashaostrikov merged 3 commits into
mainfrom
ao/bb-proj-test-add-error-msg
Jul 26, 2022
Merged

gqltest: add failure message when Bitbucket Permissions job failed to run#39428
sashaostrikov merged 3 commits into
mainfrom
ao/bb-proj-test-add-error-msg

Conversation

@sashaostrikov

Copy link
Copy Markdown
Contributor

This will add clarity to understanding the root cause of a failure.

Test plan

Existing tests should pass

@sourcegraph-bot

sourcegraph-bot commented Jul 26, 2022

Copy link
Copy Markdown
Contributor

Codenotify: Notifying subscribers in CODENOTIFY files for diff fea76e9...c5ec13e.

Notify File(s)
@unknwon dev/gqltest/bitbucket_projects_perms_sync_test.go
internal/gqltestutil/permissions.go

Comment thread internal/gqltestutil/permissions.go Outdated
return "", "", nil
} else {
return resp.Data.Jobs.Nodes[0].State, nil
return resp.Data.Jobs.Nodes[0].State, resp.Data.Jobs.Nodes[0].FailureMessage, nil

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In what cases can we get more than one here?

Could extract resp.Data.Jobs.Nodes[0] to a var to improve readability

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

in tests only if we invoke the API and schedule the job more than once

+1 for extraction, will do it now

Comment thread internal/gqltestutil/permissions.go Outdated
// GetLastBitbucketProjectPermissionJob returns a status of the most recent
// BitbucketProjectPermissionJob for given projectKey
func (c *Client) GetLastBitbucketProjectPermissionJob(projectKey string) (string, error) {
func (c *Client) GetLastBitbucketProjectPermissionJob(projectKey string) (string, string, error) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

would it make sense to add names on these returns, just so that its more obvious what the two different strings are?

@varsanojidan varsanojidan left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants