Fix up test for etag updates #32
Conversation
|
@gregkare Please review. Also, do you just hash the file content for etag? This might be problematic as metadata changes alone should trigger an ETag update. |
|
Let me check |
|
|
||
| @item_etag = @res.headers[:etag] | ||
| @old_listing = JSON.parse @old_listing_res.body | ||
| @listing = JSON.parse @listing_res.body | ||
| @item_info = @listing["items"]["test-object-simple.json"] | ||
| @old_item_info = @old_listing["items"]["test-object-simple.json"] | ||
| @item_info = @listing["items"]["nested-folder-object.json"] |
There was a problem hiding this comment.
It's comparing listing vs old_listing. Should I rename?
There was a problem hiding this comment.
No, I think I was just confused because of what's probably a bug in our server
There was a problem hiding this comment.
@skddc I'm investigating right now, I'm getting the wrong ETag
There was a problem hiding this comment.
It's not a bug in our server, this spec is wrong. before is a before :each so it's running three times (once for each it block). On the second it block the spec fails because the old_listing is actually the new one from the previous it
There was a problem hiding this comment.
I had to look it up because I'm usually using RSpec, there's an extension that adds before :all to minitest-spec (https://github.com/jeremyevans/minitest-hooks). We have two options: add this as a dependency or turn those three it blocks into one
There was a problem hiding this comment.
On a very related note, I think we should get rid of the tests dependency on each other, i.e. wipe the server after each test. This is slow but will let us keep our sanity.
There was a problem hiding this comment.
Cool, I'm running it once more with the updated code
untitaker
force-pushed
the
etag-updating-fixes
branch
from
c0baf58
to
6db146d
Compare
| @@ -81,35 +81,34 @@ def check_dir_listing_content_type(content_type) | |||
| end | |||
| end | |||
|
|
|||
| describe "PUT another JSON object" do | |||
| describe "updating that JSON object" do | |||
| before do | |||
There was a problem hiding this comment.
I think it would be better to group together the old_listing variables before the put request and the new ones afterwards. Also not all of those variables need to be instance variables (prefixed with @) because they're not used outside of the before block
untitaker
force-pushed
the
master
branch
2 times, most recently
from
be21ef3
to
fda6599
Compare
untitaker
force-pushed
the
etag-updating-fixes
branch
from
6db146d
to
20b8faa
Compare
|
👍 |
Fix up test for etag updates
No description provided.