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

Add unit test for goutil package #41

Merged
merged 5 commits into from
Sep 19, 2022

Conversation

KEINOS
Copy link
Contributor

@KEINOS KEINOS commented Sep 17, 2022

This PR will cover the gup/internal/goutil.

  • Add initial test to draft a PR
  • Add examples for public functions in goutil package to cover the basics
  • Add examples for public methods
  • Add tests for failed/error cases
  • Apply dependency injection to enable mock/monkey-patch
  • cover gouil up to 100%

@codecov
Copy link

codecov bot commented Sep 17, 2022

Codecov Report

Merging #41 (8476fb6) into main (6c7cc49) will increase coverage by 3.40%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main      #41      +/-   ##
==========================================
+ Coverage   84.40%   87.81%   +3.40%     
==========================================
  Files          10       11       +1     
  Lines         295      435     +140     
==========================================
+ Hits          249      382     +133     
- Misses         33       39       +6     
- Partials       13       14       +1     
Impacted Files Coverage Δ
internal/goutil/goutil.go 95.00% <100.00%> (ø)

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@nao1215
Copy link
Owner

nao1215 commented Sep 17, 2022

@KEINOS
Your way of making unit tests is more careful than mine. Perhaps I will be satisfied enough with the tests you write in the future.

cover gouil up to 100%

You don't have to force yourself to aim for 100% coverage.
Coverage of 80% or more is sufficient.

@KEINOS KEINOS force-pushed the fix-add-tests-for-goutil branch 2 times, most recently from 078131d to c4a15a7 Compare September 17, 2022 03:09
@KEINOS KEINOS changed the title chore: add tests for goutil Add unit test for goutil package Sep 17, 2022
@KEINOS KEINOS force-pushed the fix-add-tests-for-goutil branch 2 times, most recently from b482506 to d383f07 Compare September 18, 2022 12:50
@KEINOS KEINOS marked this pull request as ready for review September 18, 2022 12:53
@KEINOS
Copy link
Contributor Author

KEINOS commented Sep 18, 2022

@nao1215

I think it's ready! 👍 Please review it when you got time 🤞

@nao1215
Copy link
Owner

nao1215 commented Sep 18, 2022

@KEINOS
LGTM.
However, I do not know if the test will work on Windows.
Please merge the code from the main branch when you have time 🙏

Currently, the test passes on Linux, Mac, and Windows.
However, Mac and Windows may fail the test in rare cases due to bad race condition.
(There is a problem with the unit test I wrote)

@KEINOS KEINOS force-pushed the fix-add-tests-for-goutil branch from d383f07 to 797b4bd Compare September 19, 2022 00:44
@KEINOS
Copy link
Contributor Author

KEINOS commented Sep 19, 2022

Windows...

1 similar comment
@nao1215
Copy link
Owner

nao1215 commented Sep 19, 2022

Windows...

@KEINOS
Copy link
Contributor Author

KEINOS commented Sep 19, 2022

@nao1215

It seems the errors are from gup/internal/slice tests and not from goutils.

Is it OK to touch them?

Though, I don't know why it fails on Windows. It seems ok at the first glance.

@KEINOS
Copy link
Contributor Author

KEINOS commented Sep 19, 2022

@nao1215

Sorry, I was wrong about the above comment. It IS my bad.

https://github.com/nao1215/gup/actions/runs/3079250233/jobs/4975350665#step:4:253

@nao1215
Copy link
Owner

nao1215 commented Sep 19, 2022

@KEINOS
You need to fix "--- FAIL" ar "Run unit test" result.
Windows has a different separator for file paths, so the hard-coded paths must be replaced with filepath.Join().

@KEINOS
Copy link
Contributor Author

KEINOS commented Sep 19, 2022

Windows has a different separator for file paths

Linux「Windows...」
macOS「Windows...」
UNIX「Indeed」

@KEINOS KEINOS force-pushed the fix-add-tests-for-goutil branch 4 times, most recently from 3468014 to ea5ec9e Compare September 19, 2022 06:01
@KEINOS
Copy link
Contributor Author

KEINOS commented Sep 19, 2022

[MEMO]: Differences of module paths between platforms

macOS

$ go version -m "$(which gup)"
/Users/admin/go/bin/gup: go1.19.1
        path    github.com/nao1215/gup
        mod     github.com/nao1215/gup  v0.10.5 h1:kQKlfrQyNIBq4DlOKB26oLOFHStszgxSnTKyOtdMJbM=
        dep     github.com/fatih/color  v1.13.0 h1:8LOYc1KYPPmyKMuN8QV2DNRWNbLo6LZ0iLs8+mlH53w=
        dep     github.com/gen2brain/beeep      v0.0.0-20210529141713-5586760f0cc1      h1:Xh9mvwEmhbdXlRSsgn+N0zj/NqnKvpeqL08oKDHln2s=
        dep     github.com/mattn/go-colorable   v0.1.12 h1:jF+Du6AlPIjs2BiUiQlKOX0rt3SujHxPnksPKZbaA40=
        dep     github.com/mattn/go-isatty      v0.0.14 h1:yVuAays6BHfxijgZPzw+3Zlu5yQgKGP2/hcQbHb7S9Y=
        dep     github.com/spf13/cobra  v1.5.0  h1:X+jTBEBqF0bHN+9cSMgmfuvv2VHJ9ezmFNf9Y/XstYU=
        dep     github.com/spf13/pflag  v1.0.5  h1:iy+VFUOCP1a+8yFto/drg2CJ5u0yRoB7fZw3DKv/JXA=
        dep     golang.org/x/sys        v0.0.0-20220412211240-33da011f77ad      h1:ntjMns5wyP/fN65tdBD4g8J5w8n015+iIIs9rtjXkY0=
        build   -compiler=gc
        build   CGO_ENABLED=1
        build   CGO_CFLAGS=
        build   CGO_CPPFLAGS=
        build   CGO_CXXFLAGS=
        build   CGO_LDFLAGS=
        build   GOARCH=amd64
        build   GOOS=darwin
        build   GOAMD64=v1

Linux (Debian, Docker+golang:latest)

# go version -m "$(which gup)"
/go/bin/gup: go1.19.1
        path    github.com/nao1215/gup
        mod     github.com/nao1215/gup  v0.10.5 h1:kQKlfrQyNIBq4DlOKB26oLOFHStszgxSnTKyOtdMJbM=
        dep     github.com/fatih/color  v1.13.0 h1:8LOYc1KYPPmyKMuN8QV2DNRWNbLo6LZ0iLs8+mlH53w=
        dep     github.com/gen2brain/beeep      v0.0.0-20210529141713-5586760f0cc1      h1:Xh9mvwEmhbdXlRSsgn+N0zj/NqnKvpeqL08oKDHln2s=
        dep     github.com/godbus/dbus/v5       v5.0.3  h1:ZqHaoEF7TBzh4jzPmqVhE/5A1z9of6orkAe5uHoAeME=
        dep     github.com/mattn/go-colorable   v0.1.12 h1:jF+Du6AlPIjs2BiUiQlKOX0rt3SujHxPnksPKZbaA40=
        dep     github.com/mattn/go-isatty      v0.0.14 h1:yVuAays6BHfxijgZPzw+3Zlu5yQgKGP2/hcQbHb7S9Y=
        dep     github.com/spf13/cobra  v1.5.0  h1:X+jTBEBqF0bHN+9cSMgmfuvv2VHJ9ezmFNf9Y/XstYU=
        dep     github.com/spf13/pflag  v1.0.5  h1:iy+VFUOCP1a+8yFto/drg2CJ5u0yRoB7fZw3DKv/JXA=
        dep     golang.org/x/sys        v0.0.0-20220412211240-33da011f77ad      h1:ntjMns5wyP/fN65tdBD4g8J5w8n015+iIIs9rtjXkY0=
        build   -compiler=gc
        build   CGO_ENABLED=1
        build   CGO_CFLAGS=
        build   CGO_CPPFLAGS=
        build   CGO_CXXFLAGS=
        build   CGO_LDFLAGS=
        build   GOARCH=amd64
        build   GOOS=linux
        build   GOAMD64=v1

Windows

>where gup
C:\Users\micro\go\bin\gup.exe

>go version -m "C:\Users\micro\go\bin\gup.exe"
C:\Users\micro\go\bin\gup.exe: go1.19.1
        path    github.com/nao1215/gup
        mod     github.com/nao1215/gup  v0.10.5 h1:kQKlfrQyNIBq4DlOKB26oLOFHStszgxSnTKyOtdMJbM=
        dep     github.com/fatih/color  v1.13.0 h1:8LOYc1KYPPmyKMuN8QV2DNRWNbLo6LZ0iLs8+mlH53w=
        dep     github.com/gen2brain/beeep      v0.0.0-20210529141713-5586760f0cc1      h1:Xh9mvwEmhbdXlRSsgn+N0zj/NqnKvpeqL08oKDHln2s=
        dep     github.com/go-toast/toast       v0.0.0-20190211030409-01e6764cf0a4      h1:qZNfIGkIANxGv/OqtnntR4DfOY2+BgwR60cAcu/i3SE=
        dep     github.com/inconshreveable/mousetrap    v1.0.0  h1:Z8tu5sraLXCXIcARxBp/8cbvlwVa7Z1NHg9XEKhtSvM=
        dep     github.com/mattn/go-colorable   v0.1.12 h1:jF+Du6AlPIjs2BiUiQlKOX0rt3SujHxPnksPKZbaA40=
        dep     github.com/mattn/go-isatty      v0.0.14 h1:yVuAays6BHfxijgZPzw+3Zlu5yQgKGP2/hcQbHb7S9Y=
        dep     github.com/nu7hatch/gouuid      v0.0.0-20131221200532-179d4d0c4d8d      h1:VhgPp6v9qf9Agr/56bj7Y/xa04UccTW04VP0Qed4vnQ=
        dep     github.com/spf13/cobra  v1.5.0  h1:X+jTBEBqF0bHN+9cSMgmfuvv2VHJ9ezmFNf9Y/XstYU=
        dep     github.com/spf13/pflag  v1.0.5  h1:iy+VFUOCP1a+8yFto/drg2CJ5u0yRoB7fZw3DKv/JXA=
        dep     github.com/tadvi/systray        v0.0.0-20190226123456-11a2b8fa57af      h1:6yITBqGTE2lEeTPG04SN9W+iWHCRyHqlVYILiSXziwk=
        dep     golang.org/x/sys        v0.0.0-20220412211240-33da011f77ad      h1:ntjMns5wyP/fN65tdBD4g8J5w8n015+iIIs9rtjXkY0=
        build   -compiler=gc
        build   CGO_ENABLED=1
        build   CGO_CFLAGS=
        build   CGO_CPPFLAGS=
        build   CGO_CXXFLAGS=
        build   CGO_LDFLAGS=
        build   GOARCH=amd64
        build   GOOS=windows
        build   GOAMD64=v1

- Avoid to check OS dependent error message (wrap them with custom message).
- Avoid use of OS dependent directory separator.
@KEINOS KEINOS force-pushed the fix-add-tests-for-goutil branch from ea5ec9e to 8476fb6 Compare September 19, 2022 07:00
@nao1215
Copy link
Owner

nao1215 commented Sep 19, 2022

@KEINOS
Excellent, the test passed!! You are now friends with Windows.
The test I made seems to fail frequently. I will fix it next time.

@nao1215 nao1215 merged commit 4f7d83e into nao1215:main Sep 19, 2022
@KEINOS KEINOS deleted the fix-add-tests-for-goutil branch September 19, 2022 07:12
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