Functional tests don't exercise X-I-U-S for deletes

RESOLVED FIXED

Status

RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: gps, Assigned: rfkelly)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [qa+])

Attachments

(1 attachment, 1 obsolete attachment)

(Reporter)

Description

6 years ago
The Storage Service 2.0 spec says the DELETE APIs support X-If-Unmodified-Since. Yet, my JS server implementation doesn't support. Yet, the JS server passes the Python functional test suite! So, by extension, the Python functional test suite doesn't properly exercise this functionality.

So, this bug is a request to add testing of this functionality to the functional test suite.

I'm lazy and didn't verify this myself. If I'm wrong, something weird is going on (maybe those tests are getting skipped when executed against a remote server?).
Whiteboard: [qa+]
(Assignee)

Comment 1

6 years ago
Created attachment 630803 [details] [diff] [review]
patch adding more tests for X-If-Unmodified-Since

Indeed, the tests for X-If-Unmodified-Since were pretty minimal.  Patch adds more.
Assignee: nobody → rfkelly
Attachment #630803 - Flags: review?(telliott)
(Reporter)

Updated

6 years ago
Blocks: 760466
(Reporter)

Comment 2

6 years ago
When I implemented this in the JS server, one of the questions I had was how exactly this is implemented. I see two options for calculating the "server modified time:"

1) It is the newest modified time of all BSOs in the collection
2) It is the newest modified time of the set of BSOs being operated on by the request

The spec seems to indicate #1 with the language "If the collection to be acted on has been modified since the timestamp given, the request will fail." However, I implemented my JS server with #2 and the functional tests still pass!

If #1 is the proper implementation, please expand the functional tests to cover this case. If #2, then we probably need to clarify the spec (and possibly write more functional tests).
Attachment #630803 - Flags: review?(telliott) → review+
(Assignee)

Comment 3

6 years ago
#1 is indeed the intended interpretation, will add another test for this case.
(Assignee)

Comment 4

6 years ago
Created attachment 631262 [details] [diff] [review]
patch adding more tests for X-If-Unmodified-Since

updated patch attached.
Attachment #630803 - Attachment is obsolete: true
Attachment #631262 - Flags: review?(telliott)
Attachment #631262 - Flags: review?(telliott) → review+
(Assignee)

Comment 5

6 years ago
https://github.com/mozilla-services/server-syncstorage/commit/2301b198b8d6d651da615df48a98682b6f3e7d32
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.