Skip to content

Conversation

6543
Copy link
Member

@6543 6543 commented Nov 9, 2019

make test uses same locales always !

@6543
Copy link
Member Author

6543 commented Nov 9, 2019

@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
Copy link
Contributor

zeripath commented Nov 9, 2019

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

@6543
Copy link
Member Author

6543 commented Nov 9, 2019

@zeripath #8548 didn't fix my locale issue

@6543
Copy link
Member Author

6543 commented Nov 9, 2019

--- 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
Copy link
Member Author

6543 commented Nov 9, 2019

--- 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
Copy link
Member Author

6543 commented Nov 9, 2019

--- 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
Copy link
Member Author

6543 commented Nov 9, 2019

--- 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
Copy link

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
Copy link
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
Copy link
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.

@6543
Copy link
Member Author

6543 commented Nov 10, 2019

@zeripath big thanks!

@6543 6543 closed this Nov 10, 2019
zeripath added a commit that referenced this pull request Nov 11, 2019
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.
Labels
lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants