-
Notifications
You must be signed in to change notification settings - Fork 321
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
Numeric ranges #477
Open
trackpick
wants to merge
7
commits into
kohler:master
Choose a base branch
from
trackpick:numeric-ranges
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Numeric ranges #477
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
None of the existing ones test anything related to regexes. I'm about to make a bunch of changes and wanted to make sure I don't break the `%expect -w` behaviour for regexes. Reviewed-by: Yoann Desmouceaux <[email protected]>
This change prepares for my next commit in which I'll be making use of nested double braces. The biggest part of the change is that it replaces regex based matching with explicit matching. I don't think it's possible to implement this based on a single somewhat readable regex. I also added a unit test which can be run using `clicktest --self-test-mode`. Reviewed-by: Yoann Desmouceaux <[email protected]>
This change adds support for three types of integer range expressions in %expect and %ignore sections. Here is an example of each type: {{~ 5 - 15}} : allow integers from 5 to 15 (inclusive) {{~ 100 +- 5}} : allow integers from 95 to 105 (inclusive) {{~ 100 +- 5%}} : allow integers approximately 5% from 100 These can be used bare as well as inside double-braced regexes (and similarly inside %expectx and %ignorex sections) such as: {{ \d+ buckets? for {{~10 +- 10%}} euros }} I am hoping this will help developers write tests that are robust against noise by making it easy to express ranges of permitted values. Without this, developers need to handcraft equivalent regular expressions, which is cumbersome and hard to read, e.g.: {{ 9[5-9]|10[0-5] }} The current implementation only supports non-negative integers. A future change could extend this to negative numbers and floats. I added an algorithm that converts a numeric range to a regex. Ideally we'd be able to check ranges without using regexes but that would require a significant rework of clicktest. It was easier to write the algorithm. The algorithm runs in log time. Measured performance impact of this and the previous change combined by running the set of Meraki click tests that take less than 0.2 seconds each (according to parallel's joblog) in serial. I expect these to have more overhead from the clicktest script than longer running tests. As follows: The following alternates between a revert of this commit and this commit: ``` git reset --hard 6a62887e4bd while :; do git rv 6a62887e4bd b23972e0b1d git log --oneline HEAD~..HEAD >>before.txt (time for t in `perl -ane 'print if $F[3] <0.2' joblog.sorted.txt | awk '{print $11}'`; do ./scripts/clicktest $t; done) 2>&1 1>&2 |grep ^real >>before.txt git reset --hard HEAD~~ git log --oneline HEAD~..HEAD >>after.txt (time for t in `perl -ane 'print if $F[3] <0.2' joblog.sorted.txt | awk '{print $11}'`; do ./scripts/clicktest $t; done) 2>&1 1>&2 |grep ^real >>after.txt done ``` After 328 iterations, I measured an average runtime of 2.633s (before this commit) and 2.655s (after), with a standard deviation of ~6.5%. This represents an increase of 0.84%. Reviewed-by: Yoann Desmouceaux <[email protected]>
E.g. '[3-3]' can be replaced by just '3'. Making this change to make generated regexes a bit more readable. Reviewed-by: Yoann Desmouceaux <[email protected]>
..in preparation for handling floating point range expressions (next commit). This is mostly a no-op but not quite: integer ranges are handled as a special case of numeric (float/int) ranges, which support leading zeros. As a result integer range now accept leading zeros as well (as they should have before). Reviewed-by: Yoann Desmouceaux <[email protected]>
Previously, I added integer range expressions. In this commit, I expand them to "numeric range expressions", which support integer and float ranges. The change in format is pretty minimal: if any of the numbers in the format contains a decimal point, floats and integers are accepted; otherwise only integers are accepted (as before). Here is an example of the float variant of each type: {{~ 5.0 - 15 }} # note: equivalent to {{~ 5 - 15.0 }} {{~ 100 +- 5.0 }} {{~ 100 +- 5.0% }} Reviewed-by: Yoann Desmouceaux <[email protected]>
I manually updated the pod documentation inside the clicktest script and then generated clicktest.1 using `pod2man -d '' -c ''` as specified in doc/Makefile.in. A bigger diff resulted than I expected, but I believe this is the correct procedure. Reviewed-by: Yoann Desmouceaux <[email protected]>
tbarbette
approved these changes
Jan 22, 2021
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's a nice feature to have, and I guess any mean to encourage people to test stuffs is a good one.
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.
This adds a syntax for numeric ranges to clicktest. We've been running with this at Meraki since about September.
As part of this I added an algorithm that converts a numeric range to a regex. Ideally we'd be able to check ranges without using regexes but that would require a significant rework of clicktest. Figured it was easier to write the algorithm.