Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 11 additions & 6 deletions .github/workflows/test-cli.yml
Original file line number Diff line number Diff line change
Expand Up @@ -26,15 +26,20 @@ jobs:
uses: actions/setup-go@v2
with:
go-version: '1.24.4'
- name: Generate CLI and run unit tests
- name: Generate CLI
env:
GOPRIVATE: github.com/phrase/phrase-go
run: |
npm install
npm run generate.go
{
npm install
npm run generate.go
cd ./clients/cli
go mod edit -replace github.com/phrase/phrase-go/v4=../go
npm run generate.cli
} > generate.log 2>&1 || { cat generate.log; exit 1; }
- name: Build and run unit tests
run: |
cd ./clients/cli
go mod edit -replace github.com/phrase/phrase-go/v4=../go
npm run generate.cli
go build .
go test -v ./...
- name: Install Ruby and rspec
Expand All @@ -46,7 +51,7 @@ jobs:
- name: Run integration tests
run: |
cd ./clients/cli
bundle exec rspec
bundle exec rspec --format documentation
- name: License check
uses: phrase/actions/lawa-ci@v1
with:
Expand Down
27 changes: 18 additions & 9 deletions clients/cli/cmd/internal/pull.go
Original file line number Diff line number Diff line change
Expand Up @@ -273,16 +273,16 @@ func (target *Target) PullParallel(client *phrase.APIClient, cache *DownloadCach
return dlErr
}

if cache != nil {
updateCache(cache, cacheKey, response)
}

if err := copyToDestination(file, lf.Path); err != nil {
err = fmt.Errorf("%s for %s", err, lf.Path)
results[i] = downloadResult{errMsg: err.Error()}
return err
}

if cache != nil {
updateCache(cache, cacheKey, response)
}

results[i] = downloadResult{
message: lf.Message(),
path: lf.RelPath(),
Expand Down Expand Up @@ -445,23 +445,32 @@ func (target *Target) downloadSynchronously(client *phrase.APIClient, localeFile
}
}

if err := copyToDestination(file, localeFile.Path); err != nil {
return err
}

if cache != nil {
updateCache(cache, cacheKey, response)
}

return copyToDestination(file, localeFile.Path)
return nil
}

func copyToDestination(file *os.File, path string) error {
if file == nil {
return fmt.Errorf("no content to write to %s", path)
}
defer file.Close()
destFile, err := os.Create(path)
if err != nil {
return err
}
defer destFile.Close()

// The API returns a 200 with an empty body when a filter (e.g. --tags)
// matches no translations; decode() then leaves file nil with no error.
// That's a valid empty export, not a failed download, so write an empty
// file rather than treating it as an error.
if file == nil {
return nil
}
defer file.Close()
_, err = io.Copy(destFile, file)
return err
}
Expand Down
66 changes: 66 additions & 0 deletions clients/cli/cmd/internal/pull_parallel_test.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
package internal

import (
"os"
"path/filepath"
"testing"
)

Expand Down Expand Up @@ -44,3 +46,67 @@ func TestBuildDownloadOpts_TagHandling(t *testing.T) {
t.Errorf("expected tag to be empty string, got %q", opts.Tag.Value())
}
}

// A nil *os.File represents a 200 response with an empty body, which the API
// returns when a filter (e.g. --tags) matches no translations. copyToDestination
// must write an empty file for this case rather than failing, since it's a valid
// (if unusual) export result, not a failed download.
func TestCopyToDestination_NilFileWritesEmptyFile(t *testing.T) {
path := filepath.Join(t.TempDir(), "en.json")

if err := copyToDestination(nil, path); err != nil {
t.Fatalf("unexpected error: %v", err)
}

data, err := os.ReadFile(path)
if err != nil {
t.Fatalf("expected file to be created at %s: %v", path, err)
}
if len(data) != 0 {
t.Errorf("expected empty file, got %q", data)
}
}

func TestCopyToDestination_NilFileTruncatesExistingContent(t *testing.T) {
path := filepath.Join(t.TempDir(), "en.json")
if err := os.WriteFile(path, []byte(`{"hello":"world"}`), 0o644); err != nil {
t.Fatalf("failed to seed existing file: %v", err)
}

if err := copyToDestination(nil, path); err != nil {
t.Fatalf("unexpected error: %v", err)
}

data, err := os.ReadFile(path)
if err != nil {
t.Fatalf("failed to read file: %v", err)
}
if len(data) != 0 {
t.Errorf("expected previously downloaded content to be replaced with empty file, got %q", data)
}
}

func TestCopyToDestination_CopiesFileContent(t *testing.T) {
srcPath := filepath.Join(t.TempDir(), "src.json")
if err := os.WriteFile(srcPath, []byte(`{"hello":"world"}`), 0o644); err != nil {
t.Fatalf("failed to write source file: %v", err)
}
src, err := os.Open(srcPath)
if err != nil {
t.Fatalf("failed to open source file: %v", err)
}
defer src.Close()

destPath := filepath.Join(t.TempDir(), "dest.json")
if err := copyToDestination(src, destPath); err != nil {
t.Fatalf("unexpected error: %v", err)
}

data, err := os.ReadFile(destPath)
if err != nil {
t.Fatalf("failed to read dest file: %v", err)
}
if string(data) != `{"hello":"world"}` {
t.Errorf("expected copied content, got %q", data)
}
}
56 changes: 56 additions & 0 deletions clients/cli/spec/pull_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -248,6 +248,62 @@
end
end

describe "pull with tag filter matching no translations" do
let(:config) do
<<~YAML
phrase:
host: #{ENV.fetch("BASE_URL")}
project_id: "#{project_id}"
access_token: "#{token}"
pull:
targets:
- file: "#{@tmpdir}/locales/<locale_code>.yml"
params:
file_format: yml
tags: "nonexistent-tag"
YAML
end

before do
# The API responds with 200 and an empty body when a tag filter
# matches zero translations - this is not an error.
mock_set!("GET", "/projects/#{project_id}/locales/#{locale_en_id}/download",
status: 200,
body: "",
headers: { "content-type" => "application/x-yaml" }
)
mock_set!("GET", "/projects/#{project_id}/locales/#{locale_de_id}/download",
status: 200,
body: "",
headers: { "content-type" => "application/x-yaml" }
)
end

it "succeeds and writes an empty file instead of failing" do
r = run_cli("pull", config: config)

expect(r[:exit_code]).to eq(0)

en_file_path = File.join(@tmpdir, "locales", "en.yml")
de_file_path = File.join(@tmpdir, "locales", "de.yml")

expect(File.exist?(en_file_path)).to be true
expect(File.exist?(de_file_path)).to be true
expect(File.read(en_file_path)).to eq("")
expect(File.read(de_file_path)).to eq("")
end

it "succeeds with --parallel too" do
r = run_cli("pull", "--parallel", config: config)

expect(r[:exit_code]).to eq(0)

en_file_path = File.join(@tmpdir, "locales", "en.yml")
expect(File.exist?(en_file_path)).to be true
expect(File.read(en_file_path)).to eq("")
end
end

describe "pull with locale_mapping" do
let(:config) do
<<~YAML
Expand Down
Loading