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

Add integrations tests from git cli #3377

Merged
merged 6 commits into from Jan 16, 2018

Conversation

sapk
Copy link
Member

@sapk sapk commented Jan 15, 2018

Extracted from #3152 for easy review process.

@sapk sapk changed the title Add tests from git Add integrations tests from git cli Jan 15, 2018
@codecov-io
Copy link

codecov-io commented Jan 16, 2018

Codecov Report

Merging #3377 into master will increase coverage by 0.58%.
The diff coverage is 50%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master   #3377      +/-   ##
=========================================
+ Coverage   35.01%   35.6%   +0.58%     
=========================================
  Files         281     281              
  Lines       40560   40571      +11     
=========================================
+ Hits        14204   14447     +243     
+ Misses      24255   23985     -270     
- Partials     2101    2139      +38
Impacted Files Coverage Δ
models/ssh_key.go 38.46% <50%> (-0.74%) ⬇️
models/repo_list.go 65.62% <0%> (-1.57%) ⬇️
modules/setting/setting.go 46.42% <0%> (-1.03%) ⬇️
models/repo.go 43.25% <0%> (-0.13%) ⬇️
models/repo_indexer.go 46.63% <0%> (ø) ⬆️
modules/lfs/server.go 37.66% <0%> (+2.65%) ⬆️
modules/lfs/locks.go 55.72% <0%> (+3.64%) ⬆️
routers/init.go 80% <0%> (+7.27%) ⬆️
routers/private/internal.go 59.09% <0%> (+13.63%) ⬆️
... and 3 more

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 cc0c4a3...f0a4220. Read the comment docs.

@tboerger tboerger added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Jan 16, 2018
@lafriks
Copy link
Member

lafriks commented Jan 16, 2018

Remove debug but otherwise LGTM

@tboerger tboerger added lgtm/need 1 This PR needs approval from one additional maintainer to be merged. and removed lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. labels Jan 16, 2018
@sapk
Copy link
Member Author

sapk commented Jan 16, 2018

@lafriks done. I don't know why it fix the blocking situation in previous commit. (maybe related to drone ?)

@strk
Copy link
Member

strk commented Jan 16, 2018

trusted LGTM although the code is very hard to review, lots of things removed!

@tboerger tboerger added lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. and removed lgtm/need 1 This PR needs approval from one additional maintainer to be merged. labels Jan 16, 2018
@sapk
Copy link
Member Author

sapk commented Jan 16, 2018

@strk I refactor some code to be in some function like generateCommitWithNewData, commitAndPush, lockTest and that why I wanted to extract it from #3152 to "ease" the review.
This mostly duplicate existing http git clone to also do ssh tests.

@lafriks lafriks merged commit 095fb9f into go-gitea:master Jan 16, 2018
sapk added a commit to sapk-fork/gitea that referenced this pull request Jan 16, 2018
In go-gitea#3377, I put check inside all write key function. This one is useless no.
This was referenced Jan 16, 2018
lafriks pushed a commit that referenced this pull request Jan 19, 2018
In #3377, I put check inside all write key function. This one is useless no.
@sapk sapk deleted the add-tests-from-git branch May 19, 2018 23:57
@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/done This PR has enough approvals to get merged. There are no important open reservations anymore. type/testing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants