From 31eebdf9da8cd0f6afd7999175fb53cc135a1833 Mon Sep 17 00:00:00 2001 From: Amine Hilaly Date: Sun, 23 Jun 2019 17:54:38 +0200 Subject: [bridge/github] Correcte some types and add comments MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit General improvements Co-Authored-By: Michael Muré empty array check an empty array is not nil Co-Authored-By: Michael Muré --- bridge/github/export.go | 20 +++++++++++++------- bridge/github/export_test.go | 25 +++++++++++++++++++------ 2 files changed, 32 insertions(+), 13 deletions(-) (limited to 'bridge') diff --git a/bridge/github/export.go b/bridge/github/export.go index 1ab6ebca..9e394334 100644 --- a/bridge/github/export.go +++ b/bridge/github/export.go @@ -22,7 +22,7 @@ var ( ErrMissingIdentityToken = errors.New("missing identity token") ) -// githubImporter implement the Importer interface +// githubExporter implement the Exporter interface type githubExporter struct { conf core.Configuration @@ -63,8 +63,11 @@ func (ge *githubExporter) Init(conf core.Configuration) error { return nil } +// allowOrigin verify that origin is allowed to get exported. +// if the exporter was initialized with no specified origins, it will return +// true for all origins func (ge *githubExporter) allowOrigin(origin string) bool { - if ge.onlyOrigins == nil { + if len(ge.onlyOrigins) == 0 { return true } @@ -78,7 +81,7 @@ func (ge *githubExporter) allowOrigin(origin string) bool { } // getIdentityClient return an identity github api v4 client -// if no client were found it will initilize it from the known tokens map and cache it for next it use +// if no client were found it will initilize it from the known tokens map and cache it for next use func (ge *githubExporter) getIdentityClient(id string) (*githubv4.Client, error) { client, ok := ge.identityClient[id] if ok { @@ -143,6 +146,7 @@ bugLoop: return err } + // avoid calling exportBug multiple times for the same bug continue bugLoop } } @@ -163,7 +167,7 @@ func (ge *githubExporter) exportBug(b *cache.BugCache, since time.Time) error { // Special case: // if a user try to export a bug that is not already exported to Github (or imported - // from Github) and he is not the author of the bug. There is nothing we can do. + // from Github) and we do not have the token of the bug author, there is nothing we can do. // first operation is always createOp createOp := snapshot.Operations[0].(*bug.CreateOperation) @@ -184,6 +188,7 @@ func (ge *githubExporter) exportBug(b *cache.BugCache, since time.Time) error { // if we find github ID, github URL must be found too panic("expected to find github issue URL") } + // will be used to mark operation related to a bug as exported bugGithubID = githubID bugGithubURL = githubURL @@ -427,6 +432,7 @@ func (ge *githubExporter) getGithubLabelID(gc *githubv4.Client, label string) (s return "", err } + // if label id is empty, it means there is no such label in this Github repository if q.Repository.Label.ID == "" { return "", fmt.Errorf("label not found") } @@ -434,7 +440,7 @@ func (ge *githubExporter) getGithubLabelID(gc *githubv4.Client, label string) (s return q.Repository.Label.ID, nil } -func (ge *githubExporter) createGithubLabel(label, labelColor string) (string, error) { +func (ge *githubExporter) createGithubLabel(label, color string) (string, error) { url := fmt.Sprintf("%s/repos/%s/%s/labels", githubV3Url, ge.conf[keyOwner], ge.conf[keyProject]) client := &http.Client{ @@ -447,7 +453,7 @@ func (ge *githubExporter) createGithubLabel(label, labelColor string) (string, e Description string `json:"description"` }{ Name: label, - Color: labelColor, + Color: color, } data, err := json.Marshal(params) @@ -516,7 +522,7 @@ func (ge *githubExporter) getOrCreateGithubLabelID(gc *githubv4.Client, reposito return labelID, nil } - // hex color + // RGBA to hex color rgba := label.RGBA() hexColor := fmt.Sprintf("%.2x%.2x%.2x", rgba.R, rgba.G, rgba.B) diff --git a/bridge/github/export_test.go b/bridge/github/export_test.go index 85f5eb07..8f0f7869 100644 --- a/bridge/github/export_test.go +++ b/bridge/github/export_test.go @@ -161,19 +161,24 @@ func testCases(repo *cache.RepoCache, identity *cache.IdentityCache) ([]*testCas } -func TestExporter(t *testing.T) { +func TestPushPull(t *testing.T) { + // repo owner user := os.Getenv("TEST_USER") + + // token must have 'repo' and 'delete_repo' scopes token := os.Getenv("GITHUB_TOKEN_ADMIN") if token == "" { t.Skip("Env var GITHUB_TOKEN_ADMIN missing") } + // create repo backend repo := repository.CreateTestRepo(false) defer repository.CleanupTestRepos(t, repo) backend, err := cache.NewRepoCache(repo) require.NoError(t, err) + // set author identity author, err := backend.NewIdentity("test identity", "test@test.org") if err != nil { t.Fatal(err) @@ -195,13 +200,13 @@ func TestExporter(t *testing.T) { // generate project name projectName := generateRepoName() - // create repository + // create target Github repository if err := createRepository(projectName, token); err != nil { t.Fatal(err) } fmt.Println("created repository", projectName) - // delete repository before ending tests + // Make sure to remove the Github repository when the test end defer func(t *testing.T) { if err := deleteRepository(projectName, user, token); err != nil { t.Fatal(err) @@ -253,7 +258,9 @@ func TestExporter(t *testing.T) { // so number of operations should double require.Len(t, tt.bug.Snapshot().Operations, tt.numOrOp*2) + // verify operation have correcte metadata for _, op := range tt.bug.Snapshot().Operations { + // Check if the originals operations (*not* SetMetadata) are tagged properly if _, ok := op.(*bug.SetMetadataOperation); !ok { _, haveIDMetadata := op.GetMetadata(keyGithubId) require.True(t, haveIDMetadata) @@ -263,21 +270,23 @@ func TestExporter(t *testing.T) { } } + // get bug github ID bugGithubID, ok := tt.bug.Snapshot().Operations[0].GetMetadata(keyGithubId) require.True(t, ok) + // retrive bug from backendTwo importedBug, err := backendTwo.ResolveBugCreateMetadata(keyGithubId, bugGithubID) require.NoError(t, err) + // verify bug have same number of original operations require.Len(t, importedBug.Snapshot().Operations, tt.numOrOp) + // verify bugs are taged with origin=github issueOrigin, ok := importedBug.Snapshot().Operations[0].GetMetadata(keyOrigin) require.True(t, ok) require.Equal(t, issueOrigin, target) - for _, _ = range importedBug.Snapshot().Operations { - // test operations or last bug state ? - } + //TODO: maybe more tests to ensure bug final state }) } } @@ -292,7 +301,9 @@ func generateRepoName() string { return fmt.Sprintf("%s-%s", testRepoBaseName, string(b)) } +// create repository need a token with scope 'repo' func createRepository(project, token string) error { +// This function use the V3 Github API because repository creation is not supported yet on the V4 API. url := fmt.Sprintf("%s/user/repos", githubV3Url) params := struct { @@ -332,7 +343,9 @@ func createRepository(project, token string) error { return resp.Body.Close() } +// delete repository need a token with scope 'delete_repo' func deleteRepository(project, owner, token string) error { +// This function use the V3 Github API because repository removal is not supported yet on the V4 API. url := fmt.Sprintf("%s/repos/%s/%s", githubV3Url, owner, project) req, err := http.NewRequest("DELETE", url, nil) -- cgit