Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

#702 removing temporary files in tests #779

Merged
merged 3 commits into from
Apr 28, 2018
Merged

#702 removing temporary files in tests #779

merged 3 commits into from
Apr 28, 2018

Conversation

krzyk
Copy link
Contributor

@krzyk krzyk commented Apr 20, 2018

For #702
Replaced all temporary folder/files creation in tests with TemporaryFolders automatic removal of temporary data.

@0crat 0crat added the scope label Apr 20, 2018
@0crat
Copy link
Collaborator

0crat commented Apr 20, 2018

Job #779 is now in scope, role is REV

@0crat
Copy link
Collaborator

0crat commented Apr 20, 2018

This pull request #779 is assigned to @SharplEr/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

@codecov-io
Copy link

codecov-io commented Apr 20, 2018

Codecov Report

Merging #779 into master will increase coverage by 0.26%.
The diff coverage is n/a.

Impacted file tree graph

@@             Coverage Diff             @@
##             master    #779      +/-   ##
===========================================
+ Coverage     82.94%   83.2%   +0.26%     
- Complexity     1264    1279      +15     
===========================================
  Files           225     228       +3     
  Lines          3435    3489      +54     
  Branches        195     200       +5     
===========================================
+ Hits           2849    2903      +54     
  Misses          539     539              
  Partials         47      47
Impacted Files Coverage Δ Complexity Δ
src/main/java/org/cactoos/bytes/HexBytes.java 100% <0%> (ø) 7% <0%> (?)
src/main/java/org/cactoos/io/Zip.java 100% <0%> (ø) 4% <0%> (?)
src/main/java/org/cactoos/func/TimedFunc.java 100% <0%> (ø) 4% <0%> (?)

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 0bfef81...9cf876d. Read the comment docs.

@krzyk
Copy link
Contributor Author

krzyk commented Apr 21, 2018

@0crat status

@0crat
Copy link
Collaborator

0crat commented Apr 21, 2018

@0crat status (here)

@krzyk This is what I know about this job, as in §32:

  • The job #779 is in scope for 7hr
  • The role is REV
  • The job is assigned to @SharplEr/z for 7hr
  • There is a monetary reward attached
  • @SharplEr/z will get 25% of their hourly rate for this job
  • The job doesn't have any impediments
  • The budget is 15 minutes/points
  • These users are banned and won't be assigned:
    • @krzyk/z: This user reported the ticket
  • Job footprint (restricted area)

@krzyk
Copy link
Contributor Author

krzyk commented Apr 24, 2018

@SharplEr ping :)

@0crat
Copy link
Collaborator

0crat commented Apr 26, 2018

@SharplEr/z this job was assigned to you 5days ago. It will be taken away from you soon, unless you close it, see §8. Read this and this, please.

@@ -46,10 +48,16 @@
*/
@SuppressWarnings("PMD.TooManyMethods")
public final class InputStreamOfTest {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@krzyk You have missed in this class usage of File.createTempFile in methods: readsFileContent, readsFromUri, readsFromUrl, readsFromTextWithCharset.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@SharplEr Good catch, fixed.

@SharplEr
Copy link
Contributor

@krzyk I think it will be nice to add pdd todo task for adding File.createTempFile to forbidden API as in issue #704

@SharplEr
Copy link
Contributor

@krzyk In the project we have class TempFile, which use File.createTempFile inside. And TempFile is being using in tests: TeeOutputTest, WriterAsOutputStreamTest, ReaderAsBytesTest and GzipOutputTest. Looks like you can replace it too.

@krzyk
Copy link
Contributor Author

krzyk commented Apr 26, 2018

@SharplEr Updated the code, please see again.

@krzyk
Copy link
Contributor Author

krzyk commented Apr 26, 2018

@0crat status

@0crat
Copy link
Collaborator

0crat commented Apr 26, 2018

@0crat status (here)

@krzyk This is what I know about this job, as in §32:

  • The job #779 is in scope for 6days
  • The role is REV
  • The job is assigned to @SharplEr/z for 6days
  • There is a monetary reward attached
  • @SharplEr/z will get 25% of their hourly rate for this job
  • The job doesn't have any impediments
  • The budget is 15 minutes/points
  • These users are banned and won't be assigned:
    • @krzyk/z: This user reported the ticket
  • Job footprint (restricted area)

@SharplEr
Copy link
Contributor

@krzyk Okay, hope we didn't miss anything.
@llorllale Good to merge.

@krzyk
Copy link
Contributor Author

krzyk commented Apr 28, 2018

@llorllale ping

@llorllale
Copy link
Contributor

@rultor merge

@rultor
Copy link
Collaborator

rultor commented Apr 28, 2018

@rultor merge

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

@rultor rultor merged commit 9cf876d into yegor256:master Apr 28, 2018
@rultor
Copy link
Collaborator

rultor commented Apr 28, 2018

@rultor merge

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

@0crat
Copy link
Collaborator

0crat commented Apr 28, 2018

@elenavolokhova/z please review this job completed by @SharplEr/z, as in §30; the job will be fully closed and all payments will be made when the quality review is completed

@0crat 0crat removed the scope label Apr 28, 2018
@0crat
Copy link
Collaborator

0crat commented Apr 28, 2018

The job #779 is now out of scope

@0crat
Copy link
Collaborator

0crat commented Apr 28, 2018

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

@elenavolokhova
Copy link

@0crat quality good

@0crat
Copy link
Collaborator

0crat commented Apr 29, 2018

Order was finished, quality is "good": +20 point(s) just awarded to @SharplEr/z

@0crat
Copy link
Collaborator

0crat commented Apr 29, 2018

Quality review completed: +8 point(s) just awarded to @elenavolokhova/z

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants