#939 AppendTo for appending content to files #1021

BinaryIgor · 2019-01-17T22:19:44Z

As described in #939, new class AppendTo which implements Output was created. It encapsulates a File and returns an output with guarantee that new content will be appended to existing one. If file does not exist it throws exception.
.

0crat · 2019-01-17T22:22:54Z

Job #1021 is now in scope, role is REV

codecov-io · 2019-01-17T22:24:44Z

Codecov Report

Merging #1021 into master will decrease coverage by 0.02%.
The diff coverage is 71.42%.

@@             Coverage Diff              @@
##             master    #1021      +/-   ##
============================================
- Coverage     87.58%   87.55%   -0.03%     
- Complexity     1520     1522       +2     
============================================
  Files           267      268       +1     
  Lines          3897     3904       +7     
  Branches        215      215              
============================================
+ Hits           3413     3418       +5     
- Misses          435      437       +2     
  Partials         49       49

Impacted Files	Coverage Δ	Complexity Δ
src/main/java/org/cactoos/io/AppendTo.java	`71.42% <71.42%> (ø)`	`2 <2> (?)`

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 773f8e5...4cd8d50. Read the comment docs.

0crat · 2019-01-17T22:30:24Z

This pull request #1021 is assigned to @fabriciofx /z, here is why; the budget is 15 minutes, see §4; please, read §27 and when you decide to accept the changes, inform @llorllale /z (the architect) right in this ticket; if you decide that this PR should not be accepted ever, also inform the architect; this blog post will help you understand what is expected from a code reviewer; there will be no monetary reward for this job

fabriciofx

@Iprogrammerr done. Please, check it.

fabriciofx · 2019-01-18T04:09:35Z

src/main/java/org/cactoos/io/AppendTo.java

+     * @param source File to which content will be appended.
+     */
+    public AppendTo(final File source) {
+        this.source = source;


@Iprogrammerr use a different constructor parameter name than attribute name.

fabriciofx · 2019-01-18T04:24:13Z

src/test/java/org/cactoos/io/AppendToTest.java

+     * Temporary files and folders generator.
+     */
+    @Rule
+    public final TemporaryFolder folder = new TemporaryFolder();


@llorllale is there an alternative to this TemporaryFolder @Rule?

@Iprogrammerr Please, go ahead here.

fabriciofx · 2019-01-18T04:27:02Z

src/test/java/org/cactoos/io/AppendToTest.java

+     * A rule for handling an exception.
+     */
+    @Rule
+    public final ExpectedException exception = ExpectedException.none();


@Iprogrammerr remove this @Rule and use an alternative provided by cactoos-matchers to check exceptions.

fabriciofx · 2019-01-18T04:27:50Z

src/test/java/org/cactoos/io/AppendToTest.java

+    @Test
+    public void failsIfFileDoesNotExist() throws Exception {
+        final File source = new File(new RandomText(5).asString());
+        this.exception.expect(IOException.class);


@Iprogrammerr remove this and use an alternative provided by cactoos-matchers to check exceptions.

@fabriciofx There is no cactoos-matchers release with that mechanism provided(namely, Throws class).

@Iprogrammerr Let's wait for #1023.

fabriciofx · 2019-01-18T04:28:46Z

src/test/java/org/cactoos/io/AppendToTest.java

+        new OutputTo(source).stream().write(first.getBytes());
+        final String second = "efgh";
+        new AppendTo(source).stream().write(second.getBytes());
+        MatcherAssert.assertThat(


@Iprogrammerr use Assertion instead of MatcherAssert.assertThat.

@fabriciofx Same as above.

@Iprogrammerr The same above.

fabriciofx · 2019-01-18T04:29:05Z

src/test/java/org/cactoos/io/AppendToTest.java

+        final File source = this.folder.newFile();
+        final String first = "abdcd";
+        new OutputTo(source).stream().write(first.getBytes());
+        final String second = "efgh";


@Iprogrammerr let's check with some unicode text too.

BinaryIgor · 2019-01-18T19:10:35Z

@fabriciofx I have refactored, what could be refactored.

fabriciofx · 2019-01-18T19:26:57Z

@Iprogrammerr Let's wait for #1023.

fabriciofx · 2019-01-24T14:29:01Z

@Iprogrammerr #1023 has solved. Please, go ahead.

fabriciofx · 2019-01-25T12:45:22Z

@Iprogrammerr ping

BinaryIgor · 2019-01-25T21:10:47Z

@fabriciofx Refactored.

fabriciofx

@Iprogrammerr Done. Please, check out.

fabriciofx · 2019-01-25T23:36:43Z

src/test/java/org/cactoos/io/AppendToTest.java

+ * Test case for {@link AppendTo}.
+ *
+ * @since 1.0
+ * @checkstyle MagicNumber (500 line)


@Iprogrammerr Please, use MagicNumber just on line(s) where the number is used.

fabriciofx · 2019-01-25T23:37:23Z

src/test/java/org/cactoos/io/AppendToTest.java

+            () -> () -> new AppendTo(source).stream(),
+            new Throws<>(
+                new FormattedText(
+                "Can not append to %s file. It does not exist",


@Iprogrammerr Change "Can not" to "Can't".

fabriciofx · 2019-01-25T23:37:58Z

src/test/java/org/cactoos/io/AppendToTest.java

+    public void failsIfFileDoesNotExist() throws Exception {
+        final File source = new File(new RandomText(5).asString());
+        new Assertion<>(
+            "Doesn't throw exception with proper message",


@Iprogrammerr Change it to "Can't throw exception with proper message".

fabriciofx · 2019-01-25T23:39:46Z

src/test/java/org/cactoos/io/AppendToTest.java

+        final String second = "\\u25E6";
+        new AppendTo(source).stream().write(second.getBytes());
+        new Assertion<>(
+            "Does not contain expected unicode text",


@Iprogrammerr Change here to "Can't find expected unicode text content".

fabriciofx · 2019-01-25T23:40:17Z

src/test/java/org/cactoos/io/AppendToTest.java

+     * Temporary files and folders generator.
+     */
+    @Rule
+    public final TemporaryFolder folder = new TemporaryFolder();


@Iprogrammerr Please, go ahead here.

BinaryIgor · 2019-01-26T06:40:15Z

@fabriciofx I don't understand what you expect me to do with that TemporaryFolder. Could you clarify?

fabriciofx · 2019-01-26T12:14:07Z

@fabriciofx I don't understand what you expect me to do with that TemporaryFolder. Could you clarify?

@Iprogrammerr The idea is use something like I proposed in #1022 but it will delay you to close this issue. So, don' worry about it and go ahead.

BinaryIgor · 2019-01-26T21:18:42Z

@fabriciofx Ping.

fabriciofx

@Iprogrammerr Done. Please, check it.

fabriciofx · 2019-01-26T21:49:35Z

src/test/java/org/cactoos/io/AppendToTest.java

+    @Test
+    public void appendsUnicodeToFile() throws Exception {
+        final File source = this.folder.newFile();
+        final String first = "\\u0026\\u2022";


@Iprogrammerr Let's change these content to something more readble. Please, look at TeeInputFromTextTest for content examples.

fabriciofx · 2019-01-26T21:49:47Z

src/test/java/org/cactoos/io/AppendToTest.java

+        final File source = this.folder.newFile();
+        final String first = "\\u0026\\u2022";
+        new OutputTo(source).stream().write(first.getBytes());
+        final String second = "\\u25E6";


@Iprogrammerr Same above.

fabriciofx · 2019-01-26T21:50:07Z

src/test/java/org/cactoos/io/AppendToTest.java

+        new AppendTo(source).stream().write(second.getBytes());
+        new Assertion<>(
+            "Can't find expected unicode text content",
+            () -> new TextOf(source),


@Iprogrammerr Please, use InputOf instead.

fabriciofx · 2019-01-26T21:50:28Z

src/test/java/org/cactoos/io/AppendToTest.java

+        new Assertion<>(
+            "Can't find expected unicode text content",
+            () -> new TextOf(source),
+            new TextIs(new JoinedText("", first, second))


@Iprogrammerr Please, use InputHasContent instead.

BinaryIgor · 2019-01-27T11:19:28Z

@fabriciofx Done. Could you give all of your comments upfront, next time? This way both yours and my work will be much faster.

fabriciofx · 2019-01-27T11:50:39Z

@Iprogrammerr Sorry, but I've see some mistakes later.

fabriciofx · 2019-01-27T11:51:25Z

@Iprogrammerr This PR seems fine to me. Congratulations!

fabriciofx · 2019-01-27T11:51:33Z

@rultor merge

rultor · 2019-01-27T11:52:49Z

@rultor merge

@fabriciofx Thanks for your request. @llorllale Please confirm this.

llorllale · 2019-01-27T15:39:06Z

src/main/java/org/cactoos/io/AppendTo.java

+
+    @Override
+    public OutputStream stream() throws Exception {
+        if (!this.source.exists()) {


@Iprogrammerr this check is already baked into Files.newOutputStream() when using the APPEND option, so it can be removed.

llorllale · 2019-01-27T15:51:12Z

@Iprogrammerr left you one comment

BinaryIgor · 2019-01-28T21:48:14Z

@llorllale Done. Travis does not like commits from the past, same as #1026.

fabriciofx · 2019-01-30T01:45:08Z

@Iprogrammerr Travis is failing due to gitlint. Your comments should not exceed 72 chars size. More details you can check in here and here.

BinaryIgor · 2019-01-30T20:26:06Z

@fabriciofx Check build details, he is not failing on my commits.

fabriciofx · 2019-01-30T20:53:22Z

@llorllale Can you check it? To me, this PR seems fine to merge.

fabriciofx · 2019-02-01T17:41:52Z

@llorllale ping

llorllale · 2019-02-07T01:26:55Z

@Iprogrammerr please pull from master (don't rebase) and retrigger the travis build

BinaryIgor · 2019-02-08T21:29:42Z

@llorllale Done.

llorllale · 2019-02-09T13:19:01Z

@rultor merge

rultor · 2019-02-09T13:32:01Z

@rultor merge

@llorllale OK, I'll try to merge now. You can check the progress of the merge here

rultor · 2019-02-09T13:44:28Z

@rultor merge

@llorllale Done! FYI, the full log is here (took me 12min)

0crat · 2019-02-09T14:09:55Z

Code review was too long (22 days), architects (@llorllale) were penalized, see §55

0crat · 2019-02-09T14:12:11Z

The job #1021 is now out of scope

0crat · 2019-02-09T14:12:18Z

Order was finished: +15 point(s) just awarded to @fabriciofx /z

0crat · 2019-02-09T14:12:31Z

Payment to ARC for a closed pull request, as in §28: +10 point(s) just awarded to @llorllale /z

AppendTo with tests (yegor256#939)

998ca1f

0crat added the scope label Jan 17, 2019

fabriciofx reviewed Jan 18, 2019

View reviewed changes

Refactored AppendTo, added unicode file test (yegor256#939)

241a41e

igor added 2 commits January 25, 2019 21:27

Merge branch 'master' into 939

8a9d042

Refactored AppendToTest (yegor256#939)

5e93ac9

fabriciofx reviewed Jan 25, 2019

View reviewed changes

Continue to refactor AppendToTest (yegor256#939)

43b4f6d

fabriciofx reviewed Jan 26, 2019

View reviewed changes

Refactored AppendToTest (yegor256#939)

ccc8b4c

llorllale reviewed Jan 27, 2019

View reviewed changes

(yegor256#939) Refactored AppendTo and AppendToTest

3f50e1e

igor added 2 commits February 8, 2019 22:13

Merge branch 'master' into 939

6969dcf

(yegor256#939) Use Randomized instead of RandomText in AppendToTest

4cd8d50

rultor merged commit 4cd8d50 into yegor256:master Feb 9, 2019

0crat removed the scope label Feb 9, 2019

#939 AppendTo for appending content to files #1021

#939 AppendTo for appending content to files #1021

Conversation

BinaryIgor commented Jan 17, 2019

0crat commented Jan 17, 2019

codecov-io commented Jan 17, 2019 • edited Loading

Codecov Report

0crat commented Jan 17, 2019

fabriciofx left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

BinaryIgor commented Jan 18, 2019

fabriciofx commented Jan 18, 2019

fabriciofx commented Jan 24, 2019

fabriciofx commented Jan 25, 2019

BinaryIgor commented Jan 25, 2019

fabriciofx left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

BinaryIgor commented Jan 26, 2019

fabriciofx commented Jan 26, 2019 • edited Loading

BinaryIgor commented Jan 26, 2019

fabriciofx left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

BinaryIgor commented Jan 27, 2019

fabriciofx commented Jan 27, 2019 • edited Loading

fabriciofx commented Jan 27, 2019

fabriciofx commented Jan 27, 2019

rultor commented Jan 27, 2019

Choose a reason for hiding this comment

llorllale commented Jan 27, 2019

BinaryIgor commented Jan 28, 2019 • edited Loading

fabriciofx commented Jan 30, 2019

BinaryIgor commented Jan 30, 2019

fabriciofx commented Jan 30, 2019

fabriciofx commented Feb 1, 2019

llorllale commented Feb 7, 2019

BinaryIgor commented Feb 8, 2019

llorllale commented Feb 9, 2019

rultor commented Feb 9, 2019

rultor commented Feb 9, 2019

0crat commented Feb 9, 2019

0crat commented Feb 9, 2019

0crat commented Feb 9, 2019

0crat commented Feb 9, 2019

codecov-io commented Jan 17, 2019 •

edited

Loading

fabriciofx commented Jan 26, 2019 •

edited

Loading

fabriciofx commented Jan 27, 2019 •

edited

Loading

BinaryIgor commented Jan 28, 2019 •

edited

Loading