Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(3450)

Issue 47730043: FSA Adding MockFileDelegate, MockFile, MockDirectoryBuilder (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
10 years, 2 months ago by Brett
Modified:
10 years, 2 months ago
Reviewers:
pjo, mifern, ejona
CC:
connector-cr_google.com
Visibility:
Public.

Description

The purpose of these classes is to allow construction of in-memory file system structure with configuration of the files and directories to suit the tests that will be performed. MockFile represents a file or directory. It has many setters that allow the tests to define the file's configuration. MockFileDelegate is a FileDelegate implementation over the in-memory file system consisting of MockFiles.

Patch Set 1 #

Total comments: 16

Patch Set 2 : Redesign MockFile, remove MockDirectoryBuilder #

Total comments: 28

Patch Set 3 : Eric's and PJ's feedback #

Unified diffs Side-by-side diffs Delta from patch set Stats (+574 lines, -0 lines) Patch
A test/com/google/enterprise/adaptor/fs/MockFile.java View 1 2 1 chunk +427 lines, -0 lines 0 comments Download
A test/com/google/enterprise/adaptor/fs/MockFileDelegate.java View 1 2 1 chunk +147 lines, -0 lines 0 comments Download

Messages

Total messages: 16
Brett
10 years, 2 months ago (2014-01-04 01:41:10 UTC) #1
pjo
Thank you. A couple of quick comments. The functionality of having in memory files is ...
10 years, 2 months ago (2014-01-07 04:37:26 UTC) #2
Brett
https://codereview.appspot.com/47730043/diff/1/test/com/google/enterprise/adaptor/fs/MockDirectoryBuilder.java File test/com/google/enterprise/adaptor/fs/MockDirectoryBuilder.java (right): https://codereview.appspot.com/47730043/diff/1/test/com/google/enterprise/adaptor/fs/MockDirectoryBuilder.java#newcode50 test/com/google/enterprise/adaptor/fs/MockDirectoryBuilder.java:50: // Do not alter file configuration, add it to ...
10 years, 2 months ago (2014-01-08 00:59:23 UTC) #3
pjo
Thank you. https://codereview.appspot.com/47730043/diff/1/test/com/google/enterprise/adaptor/fs/MockDirectoryBuilder.java File test/com/google/enterprise/adaptor/fs/MockDirectoryBuilder.java (right): https://codereview.appspot.com/47730043/diff/1/test/com/google/enterprise/adaptor/fs/MockDirectoryBuilder.java#newcode50 test/com/google/enterprise/adaptor/fs/MockDirectoryBuilder.java:50: // Do not alter file configuration, add ...
10 years, 2 months ago (2014-01-08 01:21:33 UTC) #4
Brett
https://codereview.appspot.com/47730043/diff/1/test/com/google/enterprise/adaptor/fs/MockDirectoryBuilder.java File test/com/google/enterprise/adaptor/fs/MockDirectoryBuilder.java (right): https://codereview.appspot.com/47730043/diff/1/test/com/google/enterprise/adaptor/fs/MockDirectoryBuilder.java#newcode50 test/com/google/enterprise/adaptor/fs/MockDirectoryBuilder.java:50: // Do not alter file configuration, add it to ...
10 years, 2 months ago (2014-01-08 05:36:26 UTC) #5
pjo
Thank you. I appreciate the back and forth. https://codereview.appspot.com/47730043/diff/1/test/com/google/enterprise/adaptor/fs/MockDirectoryBuilder.java File test/com/google/enterprise/adaptor/fs/MockDirectoryBuilder.java (right): https://codereview.appspot.com/47730043/diff/1/test/com/google/enterprise/adaptor/fs/MockDirectoryBuilder.java#newcode50 test/com/google/enterprise/adaptor/fs/MockDirectoryBuilder.java:50: // ...
10 years, 2 months ago (2014-01-08 19:42:40 UTC) #6
ejona
https://codereview.appspot.com/47730043/diff/1/test/com/google/enterprise/adaptor/fs/MockFile.java File test/com/google/enterprise/adaptor/fs/MockFile.java (right): https://codereview.appspot.com/47730043/diff/1/test/com/google/enterprise/adaptor/fs/MockFile.java#newcode68 test/com/google/enterprise/adaptor/fs/MockFile.java:68: LENGTH, NEW_INPUT_STREAM, NEW_DIRECTORY_STREAM, IS_HIDDEN } On 2014/01/08 19:42:40, pjo ...
10 years, 2 months ago (2014-01-11 02:02:32 UTC) #7
pjo
I think this CL should either drastically transform into something that has tests and a ...
10 years, 2 months ago (2014-01-11 02:18:07 UTC) #8
Brett
Redesign MockFile, remove MockDirectoryBuilder
10 years, 2 months ago (2014-01-16 18:54:08 UTC) #9
pjo
Thank you. This review has a couple of specific notes about specific points in the ...
10 years, 2 months ago (2014-01-16 20:02:55 UTC) #10
ejona
MockFile seems unwieldy; I wouldn't trust many tests that use it, because I can't convince ...
10 years, 2 months ago (2014-01-17 00:18:21 UTC) #11
Brett
Eric's and PJ's feedback
10 years, 2 months ago (2014-01-18 03:11:28 UTC) #12
pjo
Thank you. Can you reply to the request to advance to a CL that has ...
10 years, 2 months ago (2014-01-18 03:23:35 UTC) #13
Brett
https://codereview.appspot.com/47730043/diff/20001/test/com/google/enterprise/adaptor/fs/MockFile.java File test/com/google/enterprise/adaptor/fs/MockFile.java (right): https://codereview.appspot.com/47730043/diff/20001/test/com/google/enterprise/adaptor/fs/MockFile.java#newcode64 test/com/google/enterprise/adaptor/fs/MockFile.java:64: private String fileContents; On 2014/01/17 00:18:21, ejona wrote: > ...
10 years, 2 months ago (2014-01-20 19:50:59 UTC) #14
ejona
The changes to delete() just further strengthen my concern that MockFile seems unwieldy. And tests. ...
10 years, 2 months ago (2014-01-21 20:19:27 UTC) #15
Brett
10 years, 2 months ago (2014-01-30 07:10:39 UTC) #16
Closed as these changes have been merged into
https://codereview.appspot.com/58560043/
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f62528b