-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
Event Generator now shows data in fixed time mode #4883
Conversation
@@ -33,7 +33,7 @@ class EventTelemetryProvider { | |||
|
|||
generateData(firstObservedTime, count, startTime, duration, name) { | |||
const millisecondsSinceStart = startTime - firstObservedTime; | |||
const utc = Math.floor(startTime / duration) * duration; | |||
const utc = startTime + (count * duration); |
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.
allows any request to show data. previous incarnation was showing data well into the future
@@ -75,15 +75,15 @@ class EventTelemetryProvider { | |||
const duration = domainObject.telemetry.duration * 1000; | |||
const size = options.size ? options.size : this.defaultSize; | |||
const data = []; | |||
const firstObservedTime = Date.now(); | |||
const firstObservedTime = options.start; |
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.
just use the start request
let count = 0; | ||
|
||
if (options.strategy === 'latest' || options.size === 1) { | ||
start = end; | ||
} | ||
|
||
while (start <= end && data.length < size) { | ||
const startTime = Date.now() + count; | ||
const startTime = options.start + count; |
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.
often the time window the user has is far in the past of Date.now
, so just use the start that was requested
@@ -1,3 +1,2 @@ | |||
const testsContext = require.context('.', true, /\/(src|platform|\.\/example)\/.*Spec.js$/); | |||
|
|||
const testsContext = require.context('.', true, /^\.\/(src|example)\/.*Spec.js$/); |
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.
Adjusted regex so that we don't pick up:
./node_modules/fooPackage/src/FooSpec.js
./node_modules/fooPackage/example/FooSpec.js
but we do pick up:
./example/eventGenerator/pluginSpec.js
As the example
directory is often released to users, it seems like it should be covered by tests
Codecov Report
@@ Coverage Diff @@
## master #4883 +/- ##
==========================================
- Coverage 50.45% 50.34% -0.11%
==========================================
Files 497 510 +13
Lines 18401 18657 +256
Branches 1657 1657
==========================================
+ Hits 9284 9393 +109
- Misses 8702 8850 +148
+ Partials 415 414 -1
Continue to review full report at Codecov.
|
.github/workflows/prcop-config.json
Outdated
@@ -3,7 +3,7 @@ | |||
{ | |||
"name": "descriptionRegexp", | |||
"config": { | |||
"regexp": "x] Testing instructions", | |||
"regexp": "(?i)x] Testing instructions", |
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.
@unlikelyzero This was tripping me up as I was using X
to check the boxes in the template instead of x
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.
thanks for fixing this. You're not the only one who was using the INCORRECT markdown format ;)
@jvigliotta to take a look. |
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 looks good to me. Nice work!
Closes #4879
Describe your changes:
Changed the event generator to use passed start time instead of using
Date.now
. Also added a test to ensure this works for dates long in the past.All Submissions:
Author Checklist
Reviewer Checklist