allure2 icon indicating copy to clipboard operation
allure2 copied to clipboard

Make results' limits configurable (fixes #347)

Open sskorol opened this issue 4 years ago • 9 comments

Context

There were several places in code with hardcoded limits for items rendered on charts. It was really inconvenient for the end-user who had to rebuild the entire project to make limits configurable.

This fix moves the actual configuration into an Aggregator interface which is implemented by key plugins. Moreover, a user can now control the limits via ALLURE_RESULTS_LIMIT env variable that could be set before the report's generation. Note that a newly added method was intentionally made static as some of the APIs that adjust the limits are also static.

Fixes #347

Checklist

sskorol avatar Jun 23 '21 12:06 sskorol

Hello, Just checking to see if there is a possibility of this PR to get merged soon and what it would take to get this merged :)

Thank you.

indradeepchowdhury avatar Nov 10 '21 14:11 indradeepchowdhury

@indradeepchowdhury as far as I remember, @baev had some concerns in terms of implementation. Need to double-check with him.

sskorol avatar Nov 11 '21 09:11 sskorol

@indradeepchowdhury as far as I remember, @baev had some concerns in terms of implementation. Need to double-check with him.

Thanks for the update @sskorol. @baev would it be possible to expedite this feature. This would be a super cool feature to have.

indradeepchowdhury avatar Nov 12 '21 05:11 indradeepchowdhury

Hi @sskorol. Thanks for jour work on providing such feature. I've got one question - do you still work on this or has it been parked?

pbandura avatar Dec 20 '21 09:12 pbandura

Hi @pbandura, I don't. When I initially pushed it, it was a fully working feature. I talked to maintainers directly. And they promised to check. Seems like they didn't have time. Now I see there's a conflict. I can resolve it. And then you will be able to build a new version with this feature from sources if it's important for you.

sskorol avatar Dec 20 '21 19:12 sskorol

Hi @sskorol I think this feature would be pretty cool and I am looking forward to using it. This feature will allow us to align the history with our Sprint duration or even let us increase the history limit to a month for proper tracking 🙂

indradeepchowdhury avatar Dec 21 '21 01:12 indradeepchowdhury

@indradeepchowdhury @pbandura force-pushed changes with conflicts' resolution. Now you can build the report on your own with this feature.

sskorol avatar Dec 21 '21 09:12 sskorol

@sskorol @indradeepchowdhury @baev Why didn't you guys apply this?

sangheee avatar Oct 05 '22 03:10 sangheee

I don't like the current implementation because:

  1. Plugin API changes -- new method introduced in Aggregator interface may create some issues for other users
  2. The env variable processing is unsafe -- You can set negative or non-number env variable and crash your report generation
  3. There are no tests provided

Besides that, our team is heavily focused on Allure 3 development right now, so this change isn't really in priority right now

baev avatar Oct 05 '22 05:10 baev