Feature api download archive #5253
Conversation
|
Coverage decreased (-14.08%) when pulling 4e2b924fd71486640eede696a069faf21efa053f on karlhungus:feature-api-download-archive into 45efba5 on gitlabhq:master. |
| @@ -225,4 +225,15 @@ | |||
| end | |||
| end | |||
|
|
|||
| describe "GET /projects/:id/repository/archive/:sha" do | |||
| it "should get the archive" do | |||
There was a problem hiding this comment.
Maybe this test should also test the data given back? Response 200 is a bit to generic i think, you agree?
There was a problem hiding this comment.
@jvanbaarsen guilty, followed the example as given by the blobs tests above. I'm assuming you mean i take the body and attempt to ungzip it?
my main reason for not unpacking the received file was the time it would add to the already slow unit tests.
There was a problem hiding this comment.
@karlhungus Maybe you can test if the data you received is a archive file?
There was a problem hiding this comment.
@jvanbaarsen this:
it "should get the archive" do
get api("/projects/#{project.id}/repository/archive", user)
response.status.should == 200
storage_path = Rails.root.join("tmp", "repositories")
file_path = project.repository.archive_repo(nil, storage_path)
File.open((file_path + 'compare'),'wb'){|f|f.write(response.body)}
FileUtils.compare_file(file_path, file_path + 'compare').should be_true
endworks... seems a bit heavy handed, what do you think?
There was a problem hiding this comment.
Yeah i agree, but what about the response.body, what type of data is that?
There was a problem hiding this comment.
Not sure what you mean (i suspect i'm being dumb) it's a tar.gz of the tip of the default branch, application/x-gzip...
There was a problem hiding this comment.
I can check the content_type, but it seems little value (and won't catch the problem that the second commit solved). Eh, i'll do that as well as above, if @randx doesn't like it i'll remove it
There was a problem hiding this comment.
@karlhungus I dont think it worth to test archive_repo method again here. Just check for correct status and content type
|
You can check for the application/x-gzip? That wat you're Sure that the data you getting back is as you expect it to be |
|
Coverage remained the same when pulling a049e4af5315580208a7014823f3da845f5aa250 on karlhungus:feature-api-download-archive into 45efba5 on gitlabhq:master. |
|
@karlhungus do you need some sort of feedback from me? |
|
@randx If you are interested in this commit, wondering your opinion on the test @jvanbaarsen and I were talking about. |
|
@karlhungus ok I'll take a look :) |
| repo = user_project.repository | ||
| ref = params[:sha] | ||
| storage_path = Rails.root.join("tmp", "repositories") | ||
|
|
|
@karlhungus ping me when ready |
defaults to HEAD Conflicts: spec/requests/api/repositories_spec.rb Change-Id: Id114aca6c59d75f18e49bf9f33809a04e010bfb6
Conflicts: doc/api/repositories.md Change-Id: I7ebc39b47ff860813d9622ba6776583536e6e384
Change-Id: I2b0752bc2593a944d42dde0ffe0ef9ce408228a5
Change-Id: Id2cae87c8cd383bc6cbcbdecf0f77c161bb82eab
|
@randx allright, think this one is good |
Feature api download archive
|
@karlhungus next time you submit a feature - you can add it to changelog and mention your name. Like this one |
|
@randx allright, i'll let @amacarthur know as well :) |
|
@karlhungus thank you |
Got a hint as to the fix from here ruby-grape/grape#412