Skip to content
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

[Resolver] node list and node detail tests #74421

Merged
merged 1 commit into from
Aug 7, 2020

Conversation

oatkiller
Copy link
Contributor

@oatkiller oatkiller commented Aug 5, 2020

Summary

  • Change the way the resolver simulator works
  • refactor resolver tree and data access layer mocks
  • Fix bug where timestamp and pid sometimes don't show in the node detail view
  • add a few tests for the panel (not done, but worth committing.)

Checklist

Delete any items that are not applicable to this PR.

For maintainers

*
* NB: the logic to short-circuit the loop is here because knowledge of state is a concern of the simulator, not tests.
*/
public async *map<R>(mapper: () => R): AsyncIterable<R> {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this replaces mapStateTransitions

const title = titles.at(index).text();
const description = descriptions.at(index).text();

// Exclude timestamp since we can't currently calculate the expected description for it from tests
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the timestamp is wrong anyway

@@ -82,27 +84,24 @@ describe('Resolver, when analyzing a tree that has 1 ancestor and 2 children', (
.first()
.simulate('click');
});
it('should render the second child node as selected, and the first child not as not selected, and the query string should indicate that the second child is selected', async () => {
it('should render the second child node as selected, and the origin as not selected, and the query string should indicate that the second child is selected', async () => {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wording was wrong, test was right.

const eventTime = event.eventTimestamp(processEvent);
const dateTime = eventTime ? formatDate(eventTime) : '';
const dateTime = eventTime === undefined ? null : formatDate(eventTime);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Show the timestamp even if it's the unix epoch

@@ -107,7 +108,7 @@ export const ProcessDetails = memo(function ProcessDetails({
commandLineEntry,
]
.filter((entry) => {
return entry.description;
return entry.description !== undefined;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

show the pid and timestamp if they are present

@oatkiller oatkiller marked this pull request as ready for review August 6, 2020 16:00
@oatkiller oatkiller requested review from a team as code owners August 6, 2020 16:00
@oatkiller oatkiller changed the title Test panel [Resolver] node list and node detail tests Aug 6, 2020
@@ -43,24 +40,11 @@ interface Metadata {
/**
* A simple mock dataAccessLayer possible that returns a tree with 0 ancestors and 2 direct children. 1 related event is returned. The parameter to `entities` is ignored.
*/
export function oneAncestorTwoChildren(
{ withRelatedEvents }: { withRelatedEvents: Iterable<[string, string]> | null } = {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed options from this function, split that behavior off into its own function.

secondChildID,
});
const withRelatedEvents: Array<[string, string]> = [
['registry', 'access'],
Copy link
Contributor

@bkimmel bkimmel Aug 6, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

❔ I was hoping to make this a little more reusable, e.g. being able to pass in the set of related events from the test,

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i was hoping for the opposite :) let's discuss in office hours?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we followed up. README with recommendations for writing mocks will be added next PR

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Build metrics

async chunks size

id value diff baseline
securitySolution 7.3MB +192.0B 7.3MB

History

  • 💚 Build #66974 succeeded dd31829da5b7da77c087cf767ed4964f55a82a48
  • 💔 Build #66942 failed 69115889e1fb09d9628042cec0d259d3060189ab
  • 💛 Build #66922 was flaky 07f66a77bafa43466dc6176895fd79edb57a225f
  • 💔 Build #66964 failed 19e6c7458d2bc69014f14779ba43a8fb2bc8416f
  • 💚 Build #66767 succeeded 72678c722ce711cc8eb42362209b55d217c39c9a

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

@oatkiller oatkiller merged commit 7dc33f9 into elastic:master Aug 7, 2020
@oatkiller oatkiller deleted the test-panel branch August 7, 2020 13:15
oatkiller pushed a commit to oatkiller/kibana that referenced this pull request Aug 7, 2020
* Change the way the resolver simulator works
* refactor resolver tree and data access layer mocks
* Fix bug where timestamp and pid sometimes don't show in the node detail view
* add a few tests for the panel (not done, but worth committing.)
@kibanamachine kibanamachine added the backport missing Added to PRs automatically when the are determined to be missing a backport. label Aug 11, 2020
@kibanamachine
Copy link
Contributor

Looks like this PR has a backport PR but it still hasn't been merged. Please merge it ASAP to keep the branches relatively in sync.

oatkiller pushed a commit that referenced this pull request Aug 11, 2020
* Change the way the resolver simulator works
* refactor resolver tree and data access layer mocks
* Fix bug where timestamp and pid sometimes don't show in the node detail view
* add a few tests for the panel (not done, but worth committing.)

Co-authored-by: Elastic Machine <[email protected]>
@kibanamachine kibanamachine removed the backport missing Added to PRs automatically when the are determined to be missing a backport. label Aug 11, 2020
const eventTime = event.eventTimestamp(processEvent);
const dateTime = eventTime ? formatDate(eventTime) : '';
const dateTime = eventTime === undefined ? null : formatDate(eventTime);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

By checking if the value is undefined we can support showing the date even if it is the unix epoch (00:00:00 UTC on 1 January 1970.)

@@ -107,7 +108,7 @@ export const ProcessDetails = memo(function ProcessDetails({
commandLineEntry,
]
.filter((entry) => {
return entry.description;
return entry.description !== undefined;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

By checking for undefined here we will show the PID of a process if it is 0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants