Skip to content

use LC_ALL=C in unit-test's#8905

Closed
6543 wants to merge 3 commits into
go-gitea:masterfrom
6543-forks:make_test_use-same-locales
Closed

use LC_ALL=C in unit-test's#8905
6543 wants to merge 3 commits into
go-gitea:masterfrom
6543-forks:make_test_use-same-locales

Conversation

@6543

@6543 6543 commented Nov 9, 2019

Copy link
Copy Markdown
Member

make test uses same locales always !

@6543

6543 commented Nov 9, 2019

Copy link
Copy Markdown
Member Author

@mrsdizzie created PR ...

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Nov 9, 2019
@zeripath

zeripath commented Nov 9, 2019

Copy link
Copy Markdown
Contributor

Is this because git commands may run in different locales and you get different error messages? If so #8548 will fix this.

@6543

6543 commented Nov 9, 2019

Copy link
Copy Markdown
Member Author

@zeripath #8548 didn't fix my locale issue

@6543

6543 commented Nov 9, 2019

Copy link
Copy Markdown
Member Author 
--- FAIL: TestRepoIsEmpty (0.02s)                                                                                                                                                                                                                                                                                             
    repo_test.go:34:                                                                                                                                                                                                                                                                                                          
                Error Trace:    repo_test.go:34                                                                                                                                                                                                                                                                               
                Error:          Received unexpected error:                                                                                                                                                                                                                                                                    
                                check empty: exit status 128 - fatal: Ihr aktueller Branch 'master' hat noch keine Commits.                                                                                                                                                                                                   
                Test:           TestRepoIsEmpty

@6543

6543 commented Nov 9, 2019

Copy link
Copy Markdown
Member Author 
--- FAIL: TestGetDiffPreviewErrors/empty_repo (0.01s)                                                                                                                                                                                                                                                                     
        diff_test.go:129:                                                                                                                                      
                Error Trace:    diff_test.go:129                                                                                                               
                Error:          Error message not equal:                                                                                                       
                                expected: "repository does not exist [id: 0, uid: 0, owner_name: , name: ]"
                                actual  : "Clone: exec(33:Clone (git clone -s --bare): /tmp/appdata135631676/tmp/local-repo/upload-283806141.git) failed: exit status 128(<nil>) stdout:  stderr: fatal: Repository '/tmp/repos629996817/error/.git' existiert nicht.\n fatal: Repository '/tmp/repos629996817/error/.git' exi
stiert nicht.\n"

@6543

6543 commented Nov 9, 2019

Copy link
Copy Markdown
Member Author 
--- FAIL: TestGetDiffPreviewErrors/bad_branch (0.02s)                                                                                                      
        diff_test.go:136: 
                Error Trace:    diff_test.go:136
                Error:          Error message not equal:
                                expected: "branch does not exist [name: bad_branch]"
                                actual  : "Clone: exec(34:Clone (git clone -s --bare): /tmp/appdata135631676/tmp/local-repo/upload-292271397.git) failed: exit status 128(<nil>) stdout:  stderr: Klone in Bare-Repository '/tmp/appdata135631676/tmp/local-repo/upload-292271397.git' ...\nfatal: Remote-Branch bad_branch ni
cht im Upstream-Repository origin gefunden\nfatal: Die Gegenseite hat unerwartet abgebrochen.\n Klone in Bare-Repository '/tmp/appdata135631676/tmp/local-repo/upload-292271397.git' ...\nfatal: Remote-Branch bad_branch nicht im Upstream-Repository origin gefunden\nfatal: Die Gegenseite hat unerwartet abgebrochen.\n"
                Test:           TestGetDiffPreviewErrors/bad_branch

@6543

6543 commented Nov 9, 2019

Copy link
Copy Markdown
Member Author 
--- FAIL: TestRelease_MirrorDelete (0.16s)
    mirror_test.go:88:                                
                Error Trace:    mirror_test.go:88              
                Error:          Not equal:              
                                expected: 1                    
                                actual  : 2             
                Test:           TestRelease_MirrorDelete

@6543 6543 changed the title use locales en_US always use LC_ALL=C in unit-test's Nov 9, 2019
@codecov-io

codecov-io commented Nov 10, 2019

Copy link
Copy Markdown

Codecov Report

Merging #8905 into master will increase coverage by 0.01%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##           master   #8905      +/-   ##
=========================================
+ Coverage   41.19%   41.2%   +0.01%     
=========================================
  Files         544     544              
  Lines       70096   70096              
=========================================
+ Hits        28877   28886       +9     
+ Misses      37519   37511       -8     
+ Partials     3700    3699       -1
Impacted Files Coverage Δ
modules/indexer/indexer.go 44.73% <0%> (-10.53%) ⬇️
routers/repo/view.go 38.59% <0%> (+0.87%) ⬆️
modules/log/event.go 65.64% <0%> (+1.02%) ⬆️
models/repo_indexer.go 68.36% <0%> (+1.81%) ⬆️
models/unit.go 67.56% <0%> (+5.4%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update bb04fb5...c1d8189. Read the comment docs.

@guillep2k

guillep2k commented Nov 10, 2019

Copy link
Copy Markdown
Member

Why not just...?

GO ?= go
SED_INPLACE := sed -i
SHASUM ?= shasum -a 256

export PATH := $($(GO) env GOPATH)/bin:$(PATH)
export LC_ALL := C                     <==== added here

@mrsdizzie

mrsdizzie commented Nov 10, 2019

Copy link
Copy Markdown
Member

Why not just...?

GO ?= go
SED_INPLACE := sed -i
SHASUM ?= shasum -a 256

export PATH := $($(GO) env GOPATH)/bin:$(PATH)
export LC_ALL := C                     <==== added here

I don't think you'd want to set that for every command run (you probably do want to keep most things in your language). This is just a particular work around for the fact that unit tests require things to be output in English and can be limited to that.

@zeripath zeripath mentioned this pull request Nov 10, 2019
Merged
@6543

6543 commented Nov 10, 2019

Copy link
Copy Markdown
Member Author

@zeripath big thanks!

@6543 6543 closed this Nov 10, 2019
zeripath added a commit that referenced this pull request Nov 11, 2019
@zeripath
Migrate temp_repo.go to use git.NewCommand (#8918)
74bb292 
This PR migrates temp_repo.go to use git.NewCommand instead creating processes by itself - this fixes the problem underlying PR #8905.

There are other places that run git outside of the controlled locale defined in #8548 but temp_repo.go is the only cause of failure of local testing in cases where English is not the default - implying that error messages from those other commands are not interpreted.

Replaces #8905
@6543 6543 deleted the make_test_use-same-locales branch January 1, 2020 23:21
@go-gitea go-gitea locked and limited conversation to collaborators Nov 24, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

No reviews

Assignees

No one assigned

Labels

lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@6543 @zeripath @codecov-io @guillep2k @mrsdizzie @GiteaBot