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

Bug 61827 - TextFile always adds newline to end of string #346

Closed
wants to merge 8 commits into from
Closed

Bug 61827 - TextFile always adds newline to end of string #346

wants to merge 8 commits into from

Conversation

ham1
Copy link
Contributor

@ham1 ham1 commented Dec 1, 2017

Description

Re-wrote getText (and setText while I was there) for TextFile because getText always appends a newline separator at the end of the file regardless if the original file has one or not.

This changes the behaviour slightly and currently breaks some tests. I'm not sure if it's the tests' fault or the change. I've "fixed" the tests, but these need reviewing by someone who knows how the JMS stuff and anything else which uses it works and any assumptions they made. It also returned the new lines of the file not the system new lines.

Motivation and Context

https://bz.apache.org/bugzilla/show_bug.cgi?id=61827

a new line is adding to the content of the file read by a JMS publisher sampler.
The issue is on the getText method of the TextFile :
=> sb.append(line + lineEnd);

How Has This Been Tested?

Added unit tests to demo the new behaviour, previous code would have failed by adding an extra newline to end of file which did not end in a new line.

Screenshots (if appropriate):

Types of changes

  • Bug fix (non-breaking change which fixes an issue)

Checklist:

  • My code follows the code style of this project.
  • I have updated the documentation accordingly.

@codecov-io
Copy link

codecov-io commented Dec 1, 2017

Codecov Report

Merging #346 into trunk will increase coverage by <.01%.
The diff coverage is 84.21%.

Impacted file tree graph

@@            Coverage Diff             @@
##             trunk    #346      +/-   ##
==========================================
+ Coverage     58.1%   58.1%   +<.01%     
- Complexity   10111   10118       +7     
==========================================
  Files         1154    1155       +1     
  Lines        73961   73955       -6     
  Branches      7342    7339       -3     
==========================================
+ Hits         42972   42974       +2     
+ Misses       28509   28499      -10     
- Partials      2480    2482       +2
Impacted Files Coverage Δ Complexity Δ
...otocol/jms/sampler/render/TextMessageRenderer.java 100% <ø> (ø) 4 <0> (ø) ⬇️
...otocol/jms/sampler/render/MessageRendererTest.java 100% <ø> (ø) 4 <0> (ø) ⬇️
test/src/org/apache/jorphan/io/TextFileSpec.groovy 100% <100%> (ø) 7 <7> (?)
.../jms/sampler/render/BinaryMessageRendererTest.java 82.97% <100%> (-0.36%) 14 <2> (ø)
...ol/jms/sampler/render/TextMessageRendererTest.java 100% <100%> (ø) 12 <1> (ø) ⬇️
.../jms/sampler/render/ObjectMessageRendererTest.java 100% <100%> (ø) 11 <0> (ø) ⬇️
.../jmeter/protocol/jms/sampler/PublisherSampler.java 61.49% <56.25%> (+0.57%) 36 <3> (ø) ⬇️
src/jorphan/org/apache/jorphan/io/TextFile.java 39.53% <75%> (-7.44%) 7 <4> (ø)
...ocol/jms/sampler/render/BinaryMessageRenderer.java 87.5% <0%> (-12.5%) 6% <0%> (ø)
...s/org/apache/jmeter/timers/PoissonRandomTimer.java 72.97% <0%> (-5.41%) 9% <0%> (-1%)
... and 3 more

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 4d8facc...d040a3a. Read the comment docs.

@max3163
Copy link
Contributor

max3163 commented Dec 1, 2017

Hi @ham1,

I just review your patch, I use exactly the same way in our environment since 1-2 weeks without any problem.

So lgtm

+1

@asfgit asfgit closed this in 4919f7e Dec 1, 2017
@ham1 ham1 deleted the bug_61827 branch December 2, 2017 00:19
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.

3 participants