Numeric ranges#477
Open
trackpick wants to merge 7 commits intokohler:masterfrom
Open
Conversation
added 7 commits
January 21, 2021 14:59
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 <ydesmouc@meraki.com>
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 <ydesmouc@meraki.com>
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 <ydesmouc@meraki.com>
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 <ydesmouc@meraki.com>
..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 <ydesmouc@meraki.com>
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 <ydesmouc@meraki.com>
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 <ydesmouc@meraki.com>
tbarbette
approved these changes
Jan 22, 2021
Collaborator
tbarbette
left a comment
tbarbette
left a comment
There was a problem hiding this comment.
That's a nice feature to have, and I guess any mean to encourage people to test stuffs is a good one.
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.