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

Do not truncate arguments for --html options #5870

Closed
wants to merge 2 commits into from
Closed

Do not truncate arguments for --html options #5870

wants to merge 2 commits into from

Conversation

adaniloff
Copy link
Contributor

@adaniloff adaniloff commented Mar 6, 2020

By running the run --html, we can have a report.html file that is generated.
However, the content of this file is truncated, thus making the report sometimes unusable.

This is a quick proposal but IMO it should not be merged as it is.
We should rather create an optional conf input (or whatever) to let the user chose if yes or not, he wants the truncate to happen, and how maximal characters he desires.

Fixes #5869

By running the `run --html`, we can have a report.html file that is generated.
However, the content of this file is truncated, thus making the report sometimes unusable.

This is a quick proposal but IMO it should not be merged as it is.
We should rather create an optional conf input (or whatever) to let the user chose if yes or not, he wants the truncate to happen, and how maximal characters he desires.
Copy link
Contributor Author

@adaniloff adaniloff left a comment

See related issue #5869

@Naktibalda
Copy link
Member

@Naktibalda Naktibalda commented Mar 11, 2020

This change looks good to me (I can't find anything that could be broken by it).
The only other place where getHtml is called is codecept g:scenarios acceptance --format html command, but I think that printing complete arguments is the right thing to do there too.

We have a few tests for HTML reports in https://github.com/Codeception/Codeception/blob/4.0/tests/cli/RunCest.php , so if you could add one for this case, it would be less likely that you change will be reverted in the future.

@adaniloff
Copy link
Contributor Author

@adaniloff adaniloff commented Mar 11, 2020

@Naktibalda I've just added an unit test since it seems to be enough. I tried to refactor a bit since this is quite a mess, but after a shot I gave up since it would have take too long for me right now.

@Naktibalda
Copy link
Member

@Naktibalda Naktibalda commented Mar 14, 2020

I merged your change to 4.0 branch: cb476ea

Thank you for this contribution.

@Naktibalda Naktibalda closed this Mar 14, 2020
@Naktibalda
Copy link
Member

@Naktibalda Naktibalda commented Mar 14, 2020

Released in 4.1.2

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.

2 participants