fix: fixes path issues on windows (#829)

* return corrent path in windows with usiing path lib * ci: run tests in windows and macos too * avoid testing mysql/psql in windows and macos * windows & macos matrix * disable gotestfmt * replace which in shell script * handle error in test db removal * fix expected path baseed on platform * add leading seprator * proper temporary storage dir and db cleanup * fix failed to create destination dir file does not exist in windows * move temp to /tmp * update temp folder * fix config tests in windows * apply patch for db * revert temp dir creation. * unify account db tests pach * remove TmpDir for sqlite tests * try to force CGO disable with enviroment variable * Remove unneeded log Co-authored-by: Felipe Martin <[email protected]> * remove unneeded comment Co-authored-by: Felipe Martin <[email protected]> * fix file path for download images * change way to create temp directory * use diffrent file name for each test * fix typo * fix absolute path in successful download image * correct filename with png * change test to download image from internet instead of local machine * remvoe unneeded import * remove os.RemoveAll(.env) * unify variable names in unit test * return CGO_ENABLED=0 * test other way to set enviroment variable * try to set enviroment variable sepratly in macos and windows * set enviroment variable before run commands in windows * fix windows test name * combine two workflow for windows and macos again * fix typo * remove env * change env path * cleanup unneeded env * general CGO_ENABLED environ * use absolute path to run fileserver instead of relative * serve file test from internet shiori repository * check file existance after download and unify varibale name from temp to tmpDir * remove unneeded log --------- Co-authored-by: Felipe M <[email protected]> Co-authored-by: Felipe Martin <[email protected]>
go-shiori · Feb 5, 2024 · 82aa1e5 · 82aa1e5
1 parent 15b2a1e
commit 82aa1e5
Show file tree

Hide file tree

Showing 12 changed files with 241 additions and 449 deletions.
diff --git a/.github/workflows/_test.yml b/.github/workflows/_test.yml
@@ -2,8 +2,11 @@ name: "Unit Tests"
 
 on: workflow_call
 
+env:
+  CGO_ENABLED: 0
+
 jobs:
-  test:
+  test-linux:
     runs-on: ubuntu-latest
     services:
       postgres:
@@ -27,7 +30,7 @@ jobs:
         ports:
         - 3306:3306
 
-    name: Go unit tests
+    name: Go unit tests (ubuntu-latest)
     steps:
     - uses: actions/checkout@b4ffde65f46336ab88eb53be808477a3936bae11 # v4.1.1
 
@@ -53,10 +56,41 @@ jobs:
       env:
         SHIORI_TEST_PG_URL: "postgres://shiori:shiori@localhost:5432/shiori?sslmode=disable"
         SHIORI_TEST_MYSQL_URL: "shiori:shiori@(localhost:3306)/shiori"
+        CGO_ENABLED: 1 # go test -race requires cgo
 
-    - run: CGO_ENABLED=0 go build -tags osusergo,netgo -ldflags="-s -w -X main.version=$(git describe --tags) -X main.date=$(date --iso-8601=seconds)"
+    - run: go build -tags osusergo,netgo -ldflags="-s -w -X main.version=$(git describe --tags) -X main.date=$(date --iso-8601=seconds)"
 
     - name: Upload coverage reports to Codecov
       uses: codecov/codecov-action@4fe8c5f003fae66aa5ebb77cfd3e7bfbbda0b6b0 # 3.1.5
       env:
         CODECOV_TOKEN: ${{ secrets.CODECOV_TOKEN }}
+
+  test-windows-macos:
+    strategy:
+      matrix:
+        os: [windows-latest, macos-latest]
+    runs-on: ${{ matrix.os }}
+    name: Go unit tests (${{ matrix.os }})
+    steps:
+    - uses: actions/checkout@b4ffde65f46336ab88eb53be808477a3936bae11 # v4.1.1
+
+    - name: Setup go
+      uses: actions/setup-go@0c52d547c9bc32b1aa3301fd7a9cb496313a4491 # v5.0.0
+      with:
+        go-version-file: ./go.mod
+
+    - uses: actions/cache@e12d46a63a90f2fae62d114769bbf2a179198b5c # 3.3.3
+      with:
+        path: |
+          ~/.cache/go-build
+          ~/go/pkg
+        key: golangci-lint.cache-{platform-arch}-{interval_number}-{go.mod_hash}
+        restore-keys: |
+          golangci-lint.cache-{interval_number}-
+          golangci-lint.cache-
+
+    - run: make unittest GO_TEST_FLAGS="-tags test_sqlite_only"
+      env:
+        CGO_ENABLED: 1 # go test -race requires cgo
+
+    - run: go build -tags osusergo,netgo -ldflags="-s -w -X main.version=$(git describe --tags) -X main.date=$(date --iso-8601=seconds)"
diff --git a/Makefile b/Makefile
@@ -5,7 +5,7 @@ BASH ?= $(shell command -v bash 2> /dev/null)
 SHIORI_DIR ?= dev-data
 
 # Testing
-GO_TEST_FLAGS ?= -v -race -count=1 -covermode=atomic -coverprofile=coverage.out
+override GO_TEST_FLAGS += -v -race -count=1 -covermode=atomic -coverprofile=coverage.out
 GOTESTFMT_FLAGS ?=
 
 # Build

diff --git a/internal/config/config_test.go b/internal/config/config_test.go
@@ -67,12 +67,7 @@ func TestReadDotEnv(t *testing.T) {
 		t.Run(testCase.name, func(t *testing.T) {
 			tmpDir, err := os.MkdirTemp("", "")
 			require.NoError(t, err)
-
-			os.Chdir(tmpDir)
-
-			t.Cleanup(func() {
-				require.NoError(t, os.RemoveAll(tmpDir))
-			})
+			require.NoError(t, os.Chdir(tmpDir))
 
 			// Write the .env file in the temporary directory
 			handler, err := os.OpenFile(".env", os.O_CREATE|os.O_WRONLY, 0655)
@@ -89,12 +84,7 @@ func TestReadDotEnv(t *testing.T) {
 	t.Run("no file", func(t *testing.T) {
 		tmpDir, err := os.MkdirTemp("", "")
 		require.NoError(t, err)
-
-		os.Chdir(tmpDir)
-
-		t.Cleanup(func() {
-			require.NoError(t, os.RemoveAll(tmpDir))
-		})
+		require.NoError(t, os.Chdir(tmpDir))
 
 		e := readDotEnv(log)
 

diff --git a/internal/core/ebook_test.go b/internal/core/ebook_test.go
@@ -21,11 +21,10 @@ func TestGenerateEbook(t *testing.T) {
 
 	t.Run("Successful ebook generate", func(t *testing.T) {
 		t.Run("valid bookmarkId that return HasEbook true", func(t *testing.T) {
-			// test cae
-			tempDir := t.TempDir()
-			dstDir := t.TempDir()
+			dstFile := "/ebook/1.epub"
+			tmpDir := t.TempDir()
 
-			deps.Domains.Storage = domains.NewStorageDomain(deps, afero.NewBasePathFs(afero.NewOsFs(), dstDir))
+			deps.Domains.Storage = domains.NewStorageDomain(deps, afero.NewBasePathFs(afero.NewOsFs(), tmpDir))
 
 			mockRequest := core.ProcessRequest{
 				Bookmark: model.BookmarkDTO{
@@ -34,138 +33,144 @@ func TestGenerateEbook(t *testing.T) {
 					HTML:     "<html><body>Example HTML</body></html>",
 					HasEbook: false,
 				},
-				DataDir:     dstDir,
+				DataDir:     tmpDir,
 				ContentType: "text/html",
 			}
-
-			bookmark, err := core.GenerateEbook(deps, mockRequest, fp.Join(tempDir, "1"))
+			bookmark, err := core.GenerateEbook(deps, mockRequest, dstFile)
 
 			assert.True(t, bookmark.HasEbook)
 			assert.NoError(t, err)
 		})
 		t.Run("ebook generate with valid BookmarkID EbookExist ImagePathExist ReturnWithHasEbookTrue", func(t *testing.T) {
-			dstDir := t.TempDir()
+			dstFile := "/ebook/2.epub"
+			tmpDir := t.TempDir()
 
-			deps.Domains.Storage = domains.NewStorageDomain(deps, afero.NewBasePathFs(afero.NewOsFs(), dstDir))
+			deps.Domains.Storage = domains.NewStorageDomain(deps, afero.NewBasePathFs(afero.NewOsFs(), tmpDir))
 
 			bookmark := model.BookmarkDTO{
 				ID:       2,
 				HasEbook: false,
 			}
 			mockRequest := core.ProcessRequest{
 				Bookmark:    bookmark,
-				DataDir:     dstDir,
+				DataDir:     tmpDir,
 				ContentType: "text/html",
 			}
 			// Create the thumbnail file
 			imagePath := model.GetThumbnailPath(&bookmark)
-			deps.Domains.Storage.FS().MkdirAll(fp.Dir(imagePath), os.ModePerm)
+			imagedirPath := fp.Dir(imagePath)
+			deps.Domains.Storage.FS().MkdirAll(imagedirPath, os.ModePerm)
 			file, err := deps.Domains.Storage.FS().Create(imagePath)
 			if err != nil {
 				t.Fatal(err)
 			}
 			defer file.Close()
 
-			bookmark, err = core.GenerateEbook(deps, mockRequest, model.GetEbookPath(&bookmark))
-			expectedImagePath := "/bookmark/2/thumb"
+			bookmark, err = core.GenerateEbook(deps, mockRequest, dstFile)
+			expectedImagePath := string(fp.Separator) + fp.Join("bookmark", "2", "thumb")
 			assert.NoError(t, err)
 			assert.True(t, bookmark.HasEbook)
 			assert.Equalf(t, expectedImagePath, bookmark.ImageURL, "Expected imageURL %s, but got %s", expectedImagePath, bookmark.ImageURL)
 		})
 		t.Run("generate ebook valid BookmarkID EbookExist ReturnHasArchiveTrue", func(t *testing.T) {
-			tempDir := t.TempDir()
-			dstDir := t.TempDir()
+			dstFile := "/ebook/3.epub"
+			tmpDir := t.TempDir()
 
-			deps.Domains.Storage = domains.NewStorageDomain(deps, afero.NewBasePathFs(afero.NewOsFs(), dstDir))
+			deps.Domains.Storage = domains.NewStorageDomain(deps, afero.NewBasePathFs(afero.NewOsFs(), tmpDir))
 
 			bookmark := model.BookmarkDTO{
 				ID:       3,
 				HasEbook: false,
 			}
 			mockRequest := core.ProcessRequest{
 				Bookmark:    bookmark,
-				DataDir:     dstDir,
+				DataDir:     tmpDir,
 				ContentType: "text/html",
 			}
 			// Create the archive file
 			archivePath := model.GetArchivePath(&bookmark)
-			deps.Domains.Storage.FS().MkdirAll(fp.Dir(archivePath), os.ModePerm)
+			archiveDirPath := fp.Dir(archivePath)
+			deps.Domains.Storage.FS().MkdirAll(archiveDirPath, os.ModePerm)
 			file, err := deps.Domains.Storage.FS().Create(archivePath)
 			if err != nil {
 				t.Fatal(err)
 			}
 			defer file.Close()
 
-			bookmark, err = core.GenerateEbook(deps, mockRequest, fp.Join(tempDir, "1"))
+			bookmark, err = core.GenerateEbook(deps, mockRequest, fp.Join(dstFile, "1"))
 			assert.True(t, bookmark.HasArchive)
 			assert.NoError(t, err)
 		})
 	})
 	t.Run("specific ebook generate case", func(t *testing.T) {
 		t.Run("invalid bookmarkId that return Error", func(t *testing.T) {
-			tempDir := t.TempDir()
+			dstFile := "/ebook/0.epub"
+			tmpDir := t.TempDir()
 			mockRequest := core.ProcessRequest{
 				Bookmark: model.BookmarkDTO{
 					ID:       0,
 					HasEbook: false,
 				},
-				DataDir:     tempDir,
+				DataDir:     tmpDir,
 				ContentType: "text/html",
 			}
 
-			bookmark, err := core.GenerateEbook(deps, mockRequest, tempDir)
+			bookmark, err := core.GenerateEbook(deps, mockRequest, dstFile)
 
 			assert.Equal(t, model.BookmarkDTO{
 				ID:       0,
 				HasEbook: false,
 			}, bookmark)
-			assert.Error(t, err)
+			assert.EqualError(t, err, "bookmark ID is not valid")
 		})
 		t.Run("ebook exist return HasEbook true", func(t *testing.T) {
-			dstDir := t.TempDir()
+			dstFile := "/ebook/1.epub"
+			tmpDir := t.TempDir()
 
-			deps.Domains.Storage = domains.NewStorageDomain(deps, afero.NewBasePathFs(afero.NewOsFs(), dstDir))
+			deps.Domains.Storage = domains.NewStorageDomain(deps, afero.NewBasePathFs(afero.NewOsFs(), tmpDir))
 
 			bookmark := model.BookmarkDTO{
 				ID:       1,
 				HasEbook: false,
 			}
 			mockRequest := core.ProcessRequest{
 				Bookmark:    bookmark,
-				DataDir:     dstDir,
+				DataDir:     tmpDir,
 				ContentType: "text/html",
 			}
 			// Create the ebook file
-			ebookFilePath := model.GetEbookPath(&bookmark)
-			deps.Domains.Storage.FS().MkdirAll(fp.Dir(ebookFilePath), os.ModePerm)
-			file, err := deps.Domains.Storage.FS().Create(ebookFilePath)
+			ebookPath := model.GetEbookPath(&bookmark)
+			ebookDirPath := fp.Dir(ebookPath)
+			deps.Domains.Storage.FS().MkdirAll(ebookDirPath, os.ModePerm)
+			file, err := deps.Domains.Storage.FS().Create(ebookPath)
 			if err != nil {
 				t.Fatal(err)
 			}
 			defer file.Close()
 
-			bookmark, err = core.GenerateEbook(deps, mockRequest, ebookFilePath)
+			bookmark, err = core.GenerateEbook(deps, mockRequest, dstFile)
 
 			assert.True(t, bookmark.HasEbook)
 			assert.NoError(t, err)
 		})
 		t.Run("generate ebook valid BookmarkID RetuenError for PDF file", func(t *testing.T) {
-			tempDir := t.TempDir()
+			dstFile := "/ebook/1.epub"
+			tmpDir := t.TempDir()
 
 			mockRequest := core.ProcessRequest{
 				Bookmark: model.BookmarkDTO{
 					ID:       1,
 					HasEbook: false,
 				},
-				DataDir:     tempDir,
+				DataDir:     tmpDir,
 				ContentType: "application/pdf",
 			}
 
-			bookmark, err := core.GenerateEbook(deps, mockRequest, tempDir)
+			bookmark, err := core.GenerateEbook(deps, mockRequest, dstFile)
 
 			assert.False(t, bookmark.HasEbook)
 			assert.Error(t, err)
-			assert.Contains(t, err.Error(), "can't create ebook for pdf")
+			assert.EqualError(t, err, "can't create ebook for pdf")
 		})
 	})
 }