-
Notifications
You must be signed in to change notification settings - Fork 87
acc: extract common repls; support order for repls #2972
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
Conversation
- Use common replacement for timestamps and numbers - Add Order property to [[Repls]]. Lowest Order is applied first. Default is 0.
#[[Repls]] | ||
#Old = '\d+\?o=\d+' | ||
#New = '[NUMBER]' | ||
|
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.
Can remove?
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.
done
[[Repls]] | ||
Old = '\?o=\[NUMID\]' | ||
New = '' | ||
Order = 1000 |
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.
Why remove both if they are stable?
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.
It's not stable, ?o= suffix is completely missing in some workspaces. Added this comment.
[[Repls]] | ||
Old = '17\d{11}' | ||
New = '"UNIX_TIME_MILLIS"' | ||
|
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.
This was quoted to keep the request log valid JSON.
I recall discussing this with @shreyas-goenka a while back.
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.
Interesting. Why does it need to be valid json?
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.
To extract relevant chunks with jq. IIRC this was the case for telemetry tests.
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 should work. Replacements are applied after script is finished running. During script run it's unprocessed.
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.
OK, then I don't recall what the issue was.
Changes
[[Repls]]
object: Order. This defines order in which replacements are applied.[TEST_JOB_ID+0]
replacement, little value and does not work when making test support both local and cloud.Why
Different tests are re-doing the same work with different approaches, this adds standard way to do this.