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

Increase test coverage, split up some files #23

Merged
merged 8 commits into from
Oct 15, 2024
Merged

Increase test coverage, split up some files #23

merged 8 commits into from
Oct 15, 2024

Conversation

cheshire137
Copy link
Member

@cheshire137 cheshire137 commented Oct 15, 2024

Given our EventReader type came from another repo:

// Forked from https://github.com/Azure/azure-sdk-for-go/blob/4661007ca1fd68b2e31f3502d4282904014fd731/sdk/ai/azopenai/event_reader.go#L18

I also wanted to bring over the tests from the same referenced repository.

This branch generally increases test coverage. I split up the types.go file by pulling out any type to its own file if it had any methods on it.

Test coverage for this branch:

% go test -race -cover ./...
        github.com/github/gh-models             coverage: 0.0% of statements
ok      github.com/github/gh-models/cmd (cached)        coverage: 78.9% of statements
ok      github.com/github/gh-models/cmd/list    (cached)        coverage: 93.1% of statements
ok      github.com/github/gh-models/cmd/run     (cached)        coverage: 47.2% of statements
ok      github.com/github/gh-models/cmd/view    (cached)        coverage: 80.0% of statements
ok      github.com/github/gh-models/internal/azuremodels        (cached)        coverage: 73.2% of statements
ok      github.com/github/gh-models/internal/sse        (cached)        coverage: 58.1% of statements
        github.com/github/gh-models/pkg/command         coverage: 0.0% of statements
        github.com/github/gh-models/pkg/util            coverage: 0.0% of statements

@cheshire137 cheshire137 self-assigned this Oct 15, 2024
@cheshire137 cheshire137 added the failing-test A CI build is not passing. label Oct 15, 2024
Only thing remaining in 'ux' package, and it's operating just on an azuremodels type.
Copy link
Member Author

Choose a reason for hiding this comment

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

Copy link
Member Author

Choose a reason for hiding this comment

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

The type is EventReader so I wanted the file name to match.

@cheshire137 cheshire137 removed the failing-test A CI build is not passing. label Oct 15, 2024
@cheshire137 cheshire137 marked this pull request as ready for review October 15, 2024 22:11
@cheshire137 cheshire137 requested a review from a team as a code owner October 15, 2024 22:11
@@ -6,7 +6,6 @@ import (

"github.com/cli/go-gh/v2/pkg/tableprinter"
"github.com/github/gh-models/internal/azuremodels"
"github.com/github/gh-models/internal/ux"
Copy link
Member Author

Choose a reason for hiding this comment

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

This package was able to go away because its two functions made more sense elsewhere, either as a method on a type rather than a standalone function (IsChatModel) or because the function dealt with types defined in another package (SortModels).

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yep, I agree that's a sensible move.

Copy link
Collaborator

@sgoedecke sgoedecke left a comment

Choose a reason for hiding this comment

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

LGTM

@cheshire137 cheshire137 merged commit d078424 into main Oct 15, 2024
6 checks passed
@cheshire137 cheshire137 deleted the test-covr branch October 15, 2024 22:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants