[jira] [Updated] (OAK-5600) Test coverage for CheckCommand
[
https://issues.apache.org/jira/browse/OAK-5600?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Andrei Dulceanu updated OAK-5600:
-
Attachment: OAK-5600-04.patch
I added a new test for validating that checking {{/a}} in a repository with 2
revisions, the first one having a consistent {{/a}} and the second one having a
corrupt {{/a}} behaves correctly. With this, IMO this issue can be closed as we
cover a few cases on both a valid and an invalid repository.
[~mduerig], could you take a look at the patch?
> Test coverage for CheckCommand
> --
>
> Key: OAK-5600
> URL: https://issues.apache.org/jira/browse/OAK-5600
> Project: Jackrabbit Oak
> Issue Type: Task
> Components: run, segment-tar
>Reporter: Andrei Dulceanu
>Assignee: Andrei Dulceanu
>Priority: Minor
> Labels: tooling
> Fix For: 1.7.0, 1.8
>
> Attachments: OAK-5600-02.patch, OAK-5600-03.patch, OAK-5600-04.patch,
> OAK-5600.patch
>
>
> We should add tests for {{o.a.j.o.r.CheckCommand}} in order to validate
> recent changes introduced by adding/removing options and their arguments (see
> OAK-5275, OAK-5276, OAK-5277, OAK-5595). There is also a new feature
> introduced by OAK-5556 (filter paths) and a refactoring in OAK-5620 which
> must be thoroughly tested in order to avoid regressions.
--
This message was sent by Atlassian JIRA
(v6.3.15#6346)
[jira] [Updated] (OAK-5600) Test coverage for CheckCommand
[
https://issues.apache.org/jira/browse/OAK-5600?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Andrei Dulceanu updated OAK-5600:
-
Attachment: OAK-5600-03.patch
I attached a new patch dealing with the inconsistent repository case. Besides
the valid revision written in the beginning, another invalid revision is
written on top. This way, the consistency check eventually finds a good
revision in the repository. The test is currently ignored because I discovered
a bug introduced by OAK-5556.
To better explain the bug I'll describe the content of the revisions:
* Valid Revision
Adds child nodes {{a}}, {{b}}, {{c}}, {{d}}, {{e}}, {{f}} with various
properties (blobs included)
* Invalid Revision
Adds child node {{z}} with some blob properties and then corrupts the {{NODE}}
record holding {{z}}.
Now when the consistency check is run, it correctly detects that the second
revision is broken, *marks the path {{/z}} as corrupt* and then continues
checking the first valid revision. Because of a check introduced for OAK-5556
[1], which tries to validate the user provided absolute paths before checking
them, the checker tries to check {{/z}} in the first revision, where of course
it can't find it. Therefore the check incorrectly fails for this revision,
although it shouldn't have to.
I was thinking about how this could be solved better. The easiest thing to do
would be to remove the check and always check the root node when a path is not
found (as done pre-OAK-5620). But then, what is the right way of handling the
case of a user who wants to check a partial path (like {{/z}}) which is corrupt
in the current revision and absent in the next one? Listing the last good
revision implies that that is a consistent revision which also contains the
path, or simply the last consistent revision?
[~mduerig], [~frm] WDYT?
If you think the patch looks good, I was thinking to commit it as is (test
ignored) and to open a new issue for the bug discovered. The test will be
enabled after solving that one.
[1]
https://github.com/apache/jackrabbit-oak/blob/trunk/oak-segment-tar/src/main/java/org/apache/jackrabbit/oak/segment/file/tooling/ConsistencyChecker.java#L328
> Test coverage for CheckCommand
> --
>
> Key: OAK-5600
> URL: https://issues.apache.org/jira/browse/OAK-5600
> Project: Jackrabbit Oak
> Issue Type: Task
> Components: run, segment-tar
>Reporter: Andrei Dulceanu
>Assignee: Andrei Dulceanu
>Priority: Minor
> Labels: tooling
> Fix For: 1.7.0, 1.8
>
> Attachments: OAK-5600-02.patch, OAK-5600-03.patch, OAK-5600.patch
>
>
> We should add tests for {{o.a.j.o.r.CheckCommand}} in order to validate
> recent changes introduced by adding/removing options and their arguments (see
> OAK-5275, OAK-5276, OAK-5277, OAK-5595). There is also a new feature
> introduced by OAK-5556 (filter paths) and a refactoring in OAK-5620 which
> must be thoroughly tested in order to avoid regressions.
--
This message was sent by Atlassian JIRA
(v6.3.15#6346)
[jira] [Updated] (OAK-5600) Test coverage for CheckCommand
[
https://issues.apache.org/jira/browse/OAK-5600?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Andrei Dulceanu updated OAK-5600:
-
Description: We should add tests for {{o.a.j.o.r.CheckCommand}} in order to
validate recent changes introduced by adding/removing options and their
arguments (see OAK-5275, OAK-5276, OAK-5277, OAK-5595). There is also a new
feature introduced by OAK-5556 (filter paths) and a refactoring in OAK-5620
which must be thoroughly tested in order to avoid regressions. (was: We should
add tests for {{o.a.j.o.r.CheckCommand}} in order to validate recent changes
introduced by adding/removing options and their arguments (see OAK-5275,
OAK-5276, OAK-5277, OAK-5595). There is also a new feature introduced by
OAK-5556 (filter paths and refactoring proposed) which must be thoroughly
tested in order to avoid regressions.)
> Test coverage for CheckCommand
> --
>
> Key: OAK-5600
> URL: https://issues.apache.org/jira/browse/OAK-5600
> Project: Jackrabbit Oak
> Issue Type: Task
> Components: run, segment-tar
>Reporter: Andrei Dulceanu
>Assignee: Andrei Dulceanu
>Priority: Minor
> Labels: tooling
> Fix For: 1.7.0, 1.8
>
> Attachments: OAK-5600-02.patch, OAK-5600.patch
>
>
> We should add tests for {{o.a.j.o.r.CheckCommand}} in order to validate
> recent changes introduced by adding/removing options and their arguments (see
> OAK-5275, OAK-5276, OAK-5277, OAK-5595). There is also a new feature
> introduced by OAK-5556 (filter paths) and a refactoring in OAK-5620 which
> must be thoroughly tested in order to avoid regressions.
--
This message was sent by Atlassian JIRA
(v6.3.15#6346)
[jira] [Updated] (OAK-5600) Test coverage for CheckCommand
[
https://issues.apache.org/jira/browse/OAK-5600?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Andrei Dulceanu updated OAK-5600:
-
Attachment: OAK-5600-02.patch
Attaching a re-worked patch, following the suggestion to use two
{{PrintWriter}}s to which to delegate writing normal/erroneous output.
[~frm], could you take a look at the new patch?
/cc [~mduerig]
> Test coverage for CheckCommand
> --
>
> Key: OAK-5600
> URL: https://issues.apache.org/jira/browse/OAK-5600
> Project: Jackrabbit Oak
> Issue Type: Task
> Components: run, segment-tar
>Reporter: Andrei Dulceanu
>Assignee: Andrei Dulceanu
>Priority: Minor
> Labels: tooling
> Fix For: 1.7.0, 1.8
>
> Attachments: OAK-5600-02.patch, OAK-5600.patch
>
>
> We should add tests for {{o.a.j.o.r.CheckCommand}} in order to validate
> recent changes introduced by adding/removing options and their arguments (see
> OAK-5275, OAK-5276, OAK-5277, OAK-5595). There is also a new feature
> introduced by OAK-5556 (filter paths and refactoring proposed) which must be
> thoroughly tested in order to avoid regressions.
--
This message was sent by Atlassian JIRA
(v6.3.15#6346)
[jira] [Updated] (OAK-5600) Test coverage for CheckCommand
[
https://issues.apache.org/jira/browse/OAK-5600?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Andrei Dulceanu updated OAK-5600:
-
Attachment: OAK-5600.patch
Attaching a partial patch for this issue, covering the checking of a valid
repository. I added 5 test cases, covering different combinations of options
and arguments, both valid and invalid. In order to validate the results of the
checking I had to parse log messages logged by {{ConsistencyChecker}}. For this
I added {{InMemoryAppender}} a mock appender which holds all messages in a
{{StringBuilder}}. I am not sure how this could be achieved in a cleaner way,
please chime in with suggestions if you have.
[~frm], [~mduerig] WDYT?
If you are ok with it, I was thinking to commit these tests for the moment, in
order to unblock OAK-5620 and OAK-5556. I will re-visit this issue afterwards,
adding some tests for a repository containing broken revisions. BTW, how does
one get a broken repository in a reproducible way?
> Test coverage for CheckCommand
> --
>
> Key: OAK-5600
> URL: https://issues.apache.org/jira/browse/OAK-5600
> Project: Jackrabbit Oak
> Issue Type: Task
> Components: run, segment-tar
>Reporter: Andrei Dulceanu
>Assignee: Andrei Dulceanu
>Priority: Minor
> Labels: tooling
> Fix For: 1.7.0, 1.8
>
> Attachments: OAK-5600.patch
>
>
> We should add tests for {{o.a.j.o.r.CheckCommand}} in order to validate
> recent changes introduced by adding/removing options and their arguments (see
> OAK-5275, OAK-5276, OAK-5277, OAK-5595). There is also a new feature
> introduced by OAK-5556 (filter paths and refactoring proposed) which must be
> thoroughly tested in order to avoid regressions.
--
This message was sent by Atlassian JIRA
(v6.3.15#6346)
[jira] [Updated] (OAK-5600) Test coverage for CheckCommand
[
https://issues.apache.org/jira/browse/OAK-5600?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Andrei Dulceanu updated OAK-5600:
-
Fix Version/s: (was: 1.6.1)
1.7.0
> Test coverage for CheckCommand
> --
>
> Key: OAK-5600
> URL: https://issues.apache.org/jira/browse/OAK-5600
> Project: Jackrabbit Oak
> Issue Type: Task
> Components: run, segment-tar
>Reporter: Andrei Dulceanu
>Assignee: Andrei Dulceanu
>Priority: Minor
> Labels: tooling
> Fix For: 1.7.0, 1.8
>
>
> We should add tests for {{o.a.j.o.r.CheckCommand}} in order to validate
> recent changes introduced by adding/removing options and their arguments (see
> OAK-5275, OAK-5276, OAK-5277, OAK-5595). There is also a new feature
> introduced by OAK-5556 (filter paths and refactoring proposed) which must be
> thoroughly tested in order to avoid regressions.
--
This message was sent by Atlassian JIRA
(v6.3.15#6346)
