Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix: Using the Latest GitHub Release type filter should return the release marked as latest #989

Merged
merged 15 commits into from
Nov 19, 2022

Conversation

olblak
Copy link
Member

@olblak olblak commented Nov 19, 2022

Fix #969

Another approach than #988

Test

To test this pull request, you can run the following commands:

cp <to_package_directory>
go test

Additional Information

Tradeoff

Potential improvement

@dduportal
Copy link
Contributor

Proposal:

query getLatestRelease($owner: String!, $repository: String!) {
  rateLimit {
    cost
    remaining
    resetAt
  }
  repository(owner: $owner, name: $repository) {
    latestRelease {
      id
      name
      tagName
    }
  }
}

when "Latest: true" WDYT?

Signed-off-by: Olblak <me@olblak.com>
Signed-off-by: Olblak <me@olblak.com>
@olblak olblak changed the title Issue/969 Fix: Using the Latest GitHub Release type filter should return the release marked as latest Nov 19, 2022
@olblak olblak added bug Something isn't working resource-githubRelease Resource of kind GitHub Release labels Nov 19, 2022
@olblak
Copy link
Member Author

olblak commented Nov 19, 2022

that

Proposal:

query getLatestRelease($owner: String!, $repository: String!) {
  rateLimit {
    cost
    remaining
    resetAt
  }
  repository(owner: $owner, name: $repository) {
    latestRelease {
      id
      name
      tagName
    }
  }
}

when "Latest: true" WDYT?

That would be a better query as it would reduce the amount of information retrieved from the API which ultimately will benefit the api limit usage

@dduportal
Copy link
Contributor

dduportal commented Nov 19, 2022

I was able to update the unit test (pkg/plugins/scms/github/release_test.go) to cover:


On the main branch (commit 482425f)

ok  	github.com/updatecli/updatecli/pkg/plugins/scms/github	0.684s	coverage: 41.3% of statements

Capture d’écran 2022-11-19 à 14 25 34


Click to view the new content for the `pkg/plugins/scms/github/release_test.go` test file
package github

import (
	"fmt"
	"testing"

	"github.com/stretchr/testify/assert"
	"github.com/stretchr/testify/require"
)

func TestSearchReleases(t *testing.T) {
	tests := []struct {
		name         string
		spec         Spec
		releaseType  ReleaseType
		mockedQuery  *releasesQuery
		mockedError  error
		wantReleases []string
		wantErr      bool
	}{
		{
			name: "Case with a mix of releases (draft, prerelease, release) and default ReleaseType filter",
			spec: Spec{
				Owner:      "updatecli",
				Repository: "updatecli",
				Token:      "superSecretTokenForJoe",
				Username:   "joe",
			},
			mockedQuery: &releasesQuery{
				RateLimit: RateLimit{
					Cost:      1,
					Remaining: 5000,
				},
				Repository: struct {
					Releases repositoryRelease `graphql:"releases(last: 100, before: $before, orderBy: $orderBy)"`
				}{
					Releases: repositoryRelease{
						TotalCount: 4,
						PageInfo: PageInfo{
							HasNextPage: false,
							EndCursor:   "Y3Vyc29yOnYyOpK5MjAyMC0wMi0xOVQyMTozMDoxMyswMTowMM4BYOFb",
						},
						Edges: []releaseEdge{
							{
								Cursor: "Y3Vyc29yOnYyOpK5MjAyMi0wMS0xOFQxOTo0Nzo1MyswMTowMM4Da8vn",
								Node: releaseNode{
									Name:    "v0.18.4 🌈",
									TagName: "v0.18.4",
									IsDraft: true,
								},
							},
							{
								Cursor: "Y3Vyc29yOnYyOpK5MjAyMi0wMS0xOFQxOTo0Nzo1MyswMTowMM4Da8vn",
								Node: releaseNode{
									Name:     "v0.18.3",
									TagName:  "v0.18.3",
									IsLatest: true,
								},
							},
							{
								Cursor: "Y3Vyc29yOnYyOpK5MjAyMS0xMS0xM1QxMjozNzoxMiswMTowMM4DK66W",
								Node: releaseNode{
									Name:         "v1.0.0",
									TagName:      "v1.0.0",
									IsPrerelease: true,
								},
							},
							{
								Cursor: "Y3Vyc29yOnYyOpK5MjAyMC0wMi0xOVQyMTozMDoxMyswMTowMM4BYOFb",
								Node: releaseNode{
									Name:    "v0.0.1",
									TagName: "v0.0.1",
								},
							},
						},
					},
				},
			},
			wantReleases: []string{"v0.0.1", "v0.18.3"},
		},
		{
			name: "Case with a mix of releases (draft, prerelease, release) and a ReleaseType with draft, prerelease and releases",
			spec: Spec{
				Owner:      "updatecli",
				Repository: "updatecli",
				Token:      "superSecretTokenForJoe",
				Username:   "joe",
			},
			releaseType: ReleaseType{
				Draft:      true,
				PreRelease: true,
				Release:    true,
			},
			mockedQuery: &releasesQuery{
				RateLimit: RateLimit{
					Cost:      1,
					Remaining: 5000,
				},
				Repository: struct {
					Releases repositoryRelease `graphql:"releases(last: 100, before: $before, orderBy: $orderBy)"`
				}{
					Releases: repositoryRelease{
						TotalCount: 4,
						PageInfo: PageInfo{
							HasNextPage: false,
							EndCursor:   "Y3Vyc29yOnYyOpK5MjAyMC0wMi0xOVQyMTozMDoxMyswMTowMM4BYOFb",
						},
						Edges: []releaseEdge{
							{
								Cursor: "Y3Vyc29yOnYyOpK5MjAyMi0wMS0xOFQxOTo0Nzo1MyswMTowMM4Da8vn",
								Node: releaseNode{
									Name:    "v0.18.4 🌈",
									TagName: "v0.18.4",
									IsDraft: true,
								},
							},
							{
								Cursor: "Y3Vyc29yOnYyOpK5MjAyMi0wMS0xOFQxOTo0Nzo1MyswMTowMM4Da8vn",
								Node: releaseNode{
									Name:     "v0.18.3",
									TagName:  "v0.18.3",
									IsLatest: true,
								},
							},
							{
								Cursor: "Y3Vyc29yOnYyOpK5MjAyMS0xMS0xM1QxMjozNzoxMiswMTowMM4DK66W",
								Node: releaseNode{
									Name:         "v1.0.0",
									TagName:      "v1.0.0",
									IsPrerelease: true,
								},
							},
							{
								Cursor: "Y3Vyc29yOnYyOpK5MjAyMC0wMi0xOVQyMTozMDoxMyswMTowMM4BYOFb",
								Node: releaseNode{
									Name:    "v0.0.1",
									TagName: "v0.0.1",
								},
							},
						},
					},
				},
			},
			wantReleases: []string{"v0.0.1", "v1.0.0", "v0.18.3", "v0.18.4"},
		},
		{
			name: "Case with a mix of releases (draft, prerelease, release) and a ReleaseType with 'Latest'",
			spec: Spec{
				Owner:      "updatecli",
				Repository: "updatecli",
				Token:      "superSecretTokenForJoe",
				Username:   "joe",
			},
			releaseType: ReleaseType{
				Latest: true,
			},
			mockedQuery: &releasesQuery{
				RateLimit: RateLimit{
					Cost:      1,
					Remaining: 5000,
				},
				Repository: struct {
					Releases repositoryRelease `graphql:"releases(last: 100, before: $before, orderBy: $orderBy)"`
				}{
					Releases: repositoryRelease{
						TotalCount: 4,
						PageInfo: PageInfo{
							HasNextPage: false,
							EndCursor:   "Y3Vyc29yOnYyOpK5MjAyMC0wMi0xOVQyMTozMDoxMyswMTowMM4BYOFb",
						},
						Edges: []releaseEdge{
							{
								Cursor: "Y3Vyc29yOnYyOpK5MjAyMi0wMS0xOFQxOTo0Nzo1MyswMTowMM4Da8vn",
								Node: releaseNode{
									Name:    "v0.18.4 🌈",
									TagName: "v0.18.4",
									IsDraft: true,
								},
							},
							{
								Cursor: "Y3Vyc29yOnYyOpK5MjAyMi0wMS0xOFQxOTo0Nzo1MyswMTowMM4Da8vn",
								Node: releaseNode{
									Name:     "v0.18.3",
									TagName:  "v0.18.3",
									IsLatest: true,
								},
							},
							{
								Cursor: "Y3Vyc29yOnYyOpK5MjAyMS0xMS0xM1QxMjozNzoxMiswMTowMM4DK66W",
								Node: releaseNode{
									Name:         "v1.0.0",
									TagName:      "v1.0.0",
									IsPrerelease: true,
								},
							},
							{
								Cursor: "Y3Vyc29yOnYyOpK5MjAyMC0wMi0xOVQyMTozMDoxMyswMTowMM4BYOFb",
								Node: releaseNode{
									Name:    "v0.0.1",
									TagName: "v0.0.1",
								},
							},
						},
					},
				},
			},
			wantReleases: []string{"v0.18.3"},
		},
		{
			name:        "Case with error returned from github query",
			mockedQuery: &releasesQuery{},
			mockedError: fmt.Errorf("Random error from GitHub API."),
			wantErr:     true,
		},
	}
	for _, tt := range tests {
		t.Run(tt.name, func(t *testing.T) {
			require.NotNil(t, tt.mockedQuery)

			sut := Github{
				Spec: tt.spec,
				client: &MockGitHubClient{
					mockedQuery: tt.mockedQuery,
					mockedErr:   tt.mockedError,
				},
			}
			tt.releaseType.Init()

			got, err := sut.SearchReleases(tt.releaseType)

			if tt.wantErr {
				assert.Error(t, err)
				return
			}

			require.NoError(t, err)

			assert.Equal(t, tt.wantReleases, got)
		})
	}
}

Result of the test on the main branch (failing as expected because of the bug I described):

time="2022-11-19T14:26:45+01:00" level=error msg="\tRandom error from GitHub API."
--- FAIL: TestSearchReleases (0.00s)
    --- FAIL: TestSearchReleases/Case_with_a_mix_of_releases_(draft,_prerelease,_release)_and_default_ReleaseType_filter (0.00s)
        /Users/dadou/workspace/updatecli/updatecli/pkg/plugins/scms/github/release_test.go:238: 
            	Error Trace:	/Users/dadou/workspace/updatecli/updatecli/pkg/plugins/scms/github/release_test.go:238
            	Error:      	Not equal: 
            	            	expected: []string{"v0.0.1", "v0.18.3"}
            	            	actual  : []string{"v0.0.1"}
            	            	
            	            	Diff:
            	            	--- Expected
            	            	+++ Actual
            	            	@@ -1,4 +1,3 @@
            	            	-([]string) (len=2) {
            	            	- (string) (len=6) "v0.0.1",
            	            	- (string) (len=7) "v0.18.3"
            	            	+([]string) (len=1) {
            	            	+ (string) (len=6) "v0.0.1"
            	            	 }
            	Test:       	TestSearchReleases/Case_with_a_mix_of_releases_(draft,_prerelease,_release)_and_default_ReleaseType_filter
    --- FAIL: TestSearchReleases/Case_with_a_mix_of_releases_(draft,_prerelease,_release)_and_a_ReleaseType_with_draft,_prerelease_and_releases (0.00s)
        /Users/dadou/workspace/updatecli/updatecli/pkg/plugins/scms/github/release_test.go:238: 
            	Error Trace:	/Users/dadou/workspace/updatecli/updatecli/pkg/plugins/scms/github/release_test.go:238
            	Error:      	Not equal: 
            	            	expected: []string{"v0.0.1", "v1.0.0", "v0.18.3", "v0.18.4"}
            	            	actual  : []string{"v0.0.1", "v1.0.0", "v0.18.4"}
            	            	
            	            	Diff:
            	            	--- Expected
            	            	+++ Actual
            	            	@@ -1,5 +1,4 @@
            	            	-([]string) (len=4) {
            	            	+([]string) (len=3) {
            	            	  (string) (len=6) "v0.0.1",
            	            	  (string) (len=6) "v1.0.0",
            	            	- (string) (len=7) "v0.18.3",
            	            	  (string) (len=7) "v0.18.4"
            	Test:       	TestSearchReleases/Case_with_a_mix_of_releases_(draft,_prerelease,_release)_and_a_ReleaseType_with_draft,_prerelease_and_releases
    --- FAIL: TestSearchReleases/Case_with_a_mix_of_releases_(draft,_prerelease,_release)_and_a_ReleaseType_with_'Latest' (0.00s)
        /Users/dadou/workspace/updatecli/updatecli/pkg/plugins/scms/github/release_test.go:238: 
            	Error Trace:	/Users/dadou/workspace/updatecli/updatecli/pkg/plugins/scms/github/release_test.go:238
            	Error:      	Not equal: 
            	            	expected: []string{"v0.18.3"}
            	            	actual  : []string{"v0.0.1", "v0.18.3"}
            	            	
            	            	Diff:
            	            	--- Expected
            	            	+++ Actual
            	            	@@ -1,2 +1,3 @@
            	            	-([]string) (len=1) {
            	            	+([]string) (len=2) {
            	            	+ (string) (len=6) "v0.0.1",
            	            	  (string) (len=7) "v0.18.3"
            	Test:       	TestSearchReleases/Case_with_a_mix_of_releases_(draft,_prerelease,_release)_and_a_ReleaseType_with_'Latest'
FAIL
FAIL	github.com/updatecli/updatecli/pkg/plugins/scms/github	0.289s
FAIL

Result on this PR (commit c964768)

ok  	github.com/updatecli/updatecli/pkg/plugins/scms/github	0.688s	coverage: 42.0% of statements

Capture d’écran 2022-11-19 à 14 31 22

@olblak
Copy link
Member Author

olblak commented Nov 19, 2022

@dduportal Did you push the updated test somewhere ? Or should I copy paste the code from your example?

@dduportal
Copy link
Contributor

@dduportal Did you push the updated test somewhere ? Or should I copy paste the code from your example?

I did not push anything: you have to copy and paste :)

@olblak
Copy link
Member Author

olblak commented Nov 19, 2022

@dduportal Thanks for your UT suggestion

Signed-off-by: Olblak <me@olblak.com>
@olblak olblak merged commit ffb2f5f into updatecli:main Nov 19, 2022
@olblak olblak deleted the issue/969 branch November 19, 2022 17:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working resource-githubRelease Resource of kind GitHub Release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants