Fix trends back/ahead result selection logic #132
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
guillaumebert
requested changes
Mar 11, 2021
There was a problem hiding this comment.
I tested with the following command on the environment https://neoload-web.neotys.perfreleng.org/ (default workspace).
The report filter with +3 seems to work, but there is a regression with filter -3 :
With this command : neoload report --type trends --filter results=+4 --template tests\resources\jinja\sample-trends-report.html.j2 --out-file report+2.html d510da5c-ad87-4d69-ad24-256f35296f01
I get these outputs :
|
SonarCloud Quality Gate failed. |
paulsbruce
requested review from
guillaumebert
and removed request for
stephanemartin
|
Kudos, SonarCloud Quality Gate passed! |
stephanemartin
pushed a commit
to stephanemartin/neoload-cli
that referenced
this pull request
Apr 15, 2021
* Fix trends back/forward result selection logic * Add logic to remove duplicates from arr_selected * Don't add base ID results again, remove dedup logic * Remove commented out dedup code because Sonar * Add logic to remove duplicates from arr_selected * Remove commented debug code; Sonar * Remove extraneous logging.debug for trends per Guillaume's suggestion * Fixed debug printout of chosen results list
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What?
Fix the report trends logic to properly calculate back/ahead values
Why?
Because there was a bug that I found during training.
How?
Update report.py :: compile_results_from_source where range should include +1
Testing
Locally, neoload --debug report --filter="results=44f1a611-bad3-451f-a7d5-9534cf62ee26|+1" --type trends --out-file ~/trends.json
Updated expected_trends json and html files for test_report_trends existing tests.
Additional Notes
I have left many logging.debug calls in the code because they are useful in prod scenarios when a customer uses a particular filter on results such as back or ahead numbers, then is confused by which results it [doesn't] pick up. Running in debug mode leaves just enough indicators to understand what the range logic is doing and why it brings back the results it brings back.