Comments and improved exception handling#2
Comments and improved exception handling#2veerkumar wants to merge 5 commits intodatadobi:masterfrom
Conversation
veerkumar
commented
Feb 10, 2026
veerkumar
commented
- Added comments on all the test cases
- Instead of traceback on test failure, print a more meaningful message.
- Cascade test failure: Added support to create a new bucket if the test fails during cleanup, and not to failthe test if teardown failed
|
@veerkumar there are some merge conflicts being reported. Do you have time to resolve those? |
|
Conflicts resolved. @pepijnve |
| bucket.create(); | ||
| } | ||
| // If we used a fallback bucket, keep using new buckets for subsequent tests (original may still exist). | ||
| if (cleanupFailedNextBucket != null) { |
There was a problem hiding this comment.
I would prefer to tackle this change separately from the documentation changes. I need to give this some more thought, but would like to get the comments merged already.
My concern here is that using a static, mutable, global variable is going to mess with test runs when, for instance, multiple tests are executed in parallel.
Would you mind splitting this off into a separate PR?
|
|
||
| /** | ||
| * Puts an object, then attempts overwrite with If-None-Match: "<etag>" (create-only if etag differs). | ||
| * Expected: Overwrite fails with 412 (or 501 if unsupported); object content remains unchanged. |
There was a problem hiding this comment.
The 501 check has been removed in the meantime so that should be removed from the comment
|
|
||
| /** | ||
| * Puts an object, then overwrites with If-Match: "<etag>" (update-only if etag matches). | ||
| * Expected: Overwrite succeeds; GET returns new content and new ETag; or 412/501 if precondition fails. |
There was a problem hiding this comment.
See earlier comment regarding 501
|
As I was going through the descriptions I started to have concerns regarding the method level comments. I think the first paragraph describing the tests makes a lot of sense. The 'expected' part is what worries me. These mostly repeat the assertions in the test and already got out of sync with the actual test. I think I would prefer to have these as inline comments on the assertions (or just have them be the assertion message), so that drift between the description and implementation is no longer possible (or less likely). |
2 participants
