-
Notifications
You must be signed in to change notification settings - Fork 21
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
feat(elements): realtime reporting #2453
Conversation
When a user has the drawer openened and we are pushing an update through, we want to test if the drawer and selected mutant stay the same.
Has some issues that I would like to resolve, for instance: - Use a better type for the mutants. - When opening the realtime report from webpack, support the use of SSE (currently it doesn't work in non integration test context since the port is unknown).
Not sure why the tests were pending, will check tomorrow |
packages/elements/testResources/realtime-reporting-example/index.html
Outdated
Show resolved
Hide resolved
packages/elements/testResources/realtime-reporting-example/mutation-report.json
Show resolved
Hide resolved
There were too many similarities between what we had and what was in the calculateMetrics version. It would still be nice to use the better reactivity that LitElement provides. Also fixed the drawer that wasn't being updated, that works now too.
@nicojs I reverted the changes we made to the rootmodel yesterday, since we were doing things that were already being done in the The changes made will also work for the coveredBy and killedBy properties now (which are also now updated when the drawer stays open 😄 ). This is not the best solution, but I feel like we should focus on reactivity in a different pr (I still want to figure out debounce, scheduledUpdates etc.). |
Not sure if this is something we want, but this causes the proces to exit even if there are pending 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.
Looks great so far. I'm not a fan of the way reactivity is built in now. I would rather see a call to requestUpdate()
instead of hacking in some solution.
packages/elements/test/integration/realtime-reporting.it.spec.ts
Outdated
Show resolved
Hide resolved
This is a better way to updat the report than what was done previously. Would also eventually allow us to make these updates more finegrained.
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.
I had limited time to review this; feel free to merge this and pick my remarks up later. I would also like to review the whole once it is done.
this.childResults.forEach((result) => result.updateParent(this)); | ||
} | ||
|
||
public updateAllMetrics() { |
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.
As I mentioned in one of our meetings: we shouldnt' need this method. In fact, we shouldn't need any public calculate
method at all. Instead, we could move all this calculation magic inside the class and only recalculate on the fly when the metrics are requested and they are dirty (undefined
).
So we start out with #metrics: TMetrics | undefined
and in the getter of metrics()
calculate when they are undefined. If we later update coverage or killed by from a child, we walk up the files and mark them dirty (setting #metrics
to undefined
).
What do you think?
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.
I assume that we would mark the #metrics
undefined when we update the state of a mutant too? How does it work in that case?
Should we create a release with the tag realtime-beta
when this gets merged?
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.
If we feel confident enough to merge we should feel confident enough to release. No need for a beta release, we don't have a release process for beta's in mutation-testing elements anyway.
In the future we should do updates in the way that was suggested: #2453 (comment)
Realtime reporting
With this PR the report now supports realtime reporting. This means that users no longer have to wait for the testing proces to complete, but can get updates while a mutation testing is in progress.
Issues
This PR has some issues that could be resolved now or at a later time:
medium: The entire report is updated now, we would like to only update what's necessary (this could be done later).(later)Note
This was tested using the implementation of Stryker.NET, although this does not support the test tab yet. This is something I need to test.closes #2434