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

Added maximum string length constraint. (#449) #452

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

lucascz37
Copy link

Adds maxStringLengthConstraint field to personalize the readStreamConstraints of JacksonCore. By default the maxStringLength of the StreamReadConstraint is 20_000_000, from what I understood if the json files surpass that size it will throw the error described in issue #449 .

Testing done

I executed three builds with .json files from my tests.

  1. Setting the maxStringLengthConstraint to 25_000_000.
  2. Default plugin execution leaving the maxStringLengthConstraint default value of 20_000_000.
  3. Limiting the maxStringLengthConstraint to 20. (Forcing to throw the same error as not a valid Cucumber report! String length (20051119) exceeds the maximum length (20000000) #449 )

Relevant links:
https://github.com/FasterXML/jackson/wiki/Jackson-Release-2.15.2
FasterXML/jackson-core#1014

Tests screenshots:
Test 1:
Test-1
Test-1 (2)

Test 2:
Test-2 (2)
Test-2

Test 3:
Test-3 (2)
Test-3

Submitter checklist

@lucascz37 lucascz37 requested a review from a team as a code owner December 6, 2023 18:38
@@ -78,6 +79,7 @@ public class CucumberReportPublisher extends Recorder implements SimpleBuildStep
private boolean undefinedAsNotFailingStatus;

private int trendsLimit;
private int maxStringLengthConstraint = 20_000_000;
Copy link
Member

Choose a reason for hiding this comment

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

nice,
would be perfect having reference to the code from that value was taken

Copy link
Author

Choose a reason for hiding this comment

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

@@ -559,6 +570,8 @@ private void generateReport(Run<?, ?> build, FilePath workspace, TaskListener li

setFailingStatuses(configuration);

StreamReadConstraints.overrideDefaultStreamReadConstraints(StreamReadConstraints.builder().maxStringLength(maxStringLengthConstraint).build());
Copy link
Member

Choose a reason for hiding this comment

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

that should be implemented in core component https://github.com/damianszczepanik/cucumber-reporting

Copy link
Author

Choose a reason for hiding this comment

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

On it.

@damianszczepanik
Copy link
Member

@lucascz37 ping

@damianszczepanik
Copy link
Member

Are you @lucascz37 going to work on this ?

@lucascz37
Copy link
Author

Hi @damianszczepanik sorry for the delay, I was on vacation, I will work on this in the evening today

<f:entry
title="${%maxStringLengthConstraint.title}"
field="maxStringLengthConstraint">
<f:textbox default="20000000"/>
Copy link

@felipecrs felipecrs Feb 21, 2024

Choose a reason for hiding this comment

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

Perhaps we should consider bumping it a little by default. Maybe 25M? This would cover the needs of all the people at #449.

What do you think, @damianszczepanik?

In such case, maybe making it configurable isn't even required (unless someone makes a case where a higher constraint is needed).

Copy link

Choose a reason for hiding this comment

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

@damianszczepanik , would you consider this simple fix from @felipecrs for #449?

Choose a reason for hiding this comment

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

@damianszczepanik, if you agree, I can send a PR for it.

Copy link
Member

Choose a reason for hiding this comment

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

IMO solution should be implemented first in https://github.com/damianszczepanik/cucumber-reporting

@BHAARATH002
Copy link

Hi is there any updates on this PR?
I'm still getting the error:
Caused by: com.fasterxml.jackson.core.exc.StreamConstraintsException: String value length (20054016) exceeds the maximum allowed (20000000, from StreamReadConstraints.getMaxStringLength())
My cucmuber report is 5.8.3 version running in the Jenkins

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.

5 participants