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

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

Closed
wants to merge 3 commits into from

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