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

[Security Solution] Template unit tests #72399

Merged
merged 12 commits into from
Jul 28, 2020

Conversation

angorayc
Copy link
Contributor

@angorayc angorayc commented Jul 20, 2020

Summary

  1. Adding more unit tests for import timelines' api.
  2. Fix known issue: Unexpected errors occurs when creating template timeline via import without templateTimelineId / templateTimelineVersion. - Enhancement for timeline templates #69972

To verify this PR:

  1. Download timelines_export.txt

  2. Change .txt into .ndjson

  3. Import the file and see if it's created successfully as a custom template.

Checklist

Delete any items that are not applicable to this PR.

For maintainers

@angorayc angorayc requested review from a team as code owners July 20, 2020 09:00
@elasticmachine
Copy link
Contributor

Pinging @elastic/siem (Team:SIEM)

@angorayc angorayc added the release_note:skip Skip the PR/issue when compiling release notes label Jul 20, 2020
@angorayc angorayc closed this Jul 20, 2020
@angorayc angorayc reopened this Jul 21, 2020
@apmmachine
Copy link
Contributor

💚 Build Succeeded

Pipeline View Test View Changes Artifacts preview

Expand to view the summary

Build stats

  • Build Cause: [Pull request #72399 updated]

  • Start Time: 2020-07-21T10:41:32.515+0000

  • Duration: 4 min 20 sec

@angorayc angorayc mentioned this pull request Jul 21, 2020
11 tasks
@angorayc
Copy link
Contributor Author

@elasticmachine merge upstream

const templateTimelineVersion = this.templateTimelineObject?.getVersion;
return (
templateTimelineVersion == null ||
(isInteger(templateTimelineVersion) && !this.isTemplateVersionConflict)
Copy link
Member

@cnasikas cnasikas Jul 22, 2020

Choose a reason for hiding this comment

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

If the templateTimelineVersion is of type string but a number (example: '123') then the function will return false. Is that intended?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, It only takes number as templateTimelineVersion.

@angorayc
Copy link
Contributor Author

@elasticmachine merge upstream

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Build metrics

async chunks size

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

History

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

Copy link
Member

@cnasikas cnasikas left a comment

Choose a reason for hiding this comment

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

LGTM! Great job with those tests!

@angorayc angorayc merged commit 7a10077 into elastic:master Jul 28, 2020
angorayc added a commit to angorayc/kibana that referenced this pull request Jul 28, 2020
* add unit test for failure cases

* add unit tests

* update wording

* fix error when update template without ttid or ttversion

* fix unit test

* add comment

* review

Co-authored-by: Elastic Machine <[email protected]>
angorayc added a commit to angorayc/kibana that referenced this pull request Jul 28, 2020
* add unit test for failure cases

* add unit tests

* update wording

* fix error when update template without ttid or ttversion

* fix unit test

* add comment

* review

Co-authored-by: Elastic Machine <[email protected]>
angorayc added a commit that referenced this pull request Jul 28, 2020
* add unit test for failure cases

* add unit tests

* update wording

* fix error when update template without ttid or ttversion

* fix unit test

* add comment

* review

Co-authored-by: Elastic Machine <[email protected]>

Co-authored-by: Elastic Machine <[email protected]>
angorayc added a commit that referenced this pull request Jul 28, 2020
* add unit test for failure cases

* add unit tests

* update wording

* fix error when update template without ttid or ttversion

* fix unit test

* add comment

* review

Co-authored-by: Elastic Machine <[email protected]>

Co-authored-by: Elastic Machine <[email protected]>
@MindyRS MindyRS added the Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc. label Sep 23, 2021
@elasticmachine
Copy link
Contributor

Pinging @elastic/security-solution (Team: SecuritySolution)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release_note:skip Skip the PR/issue when compiling release notes Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc. Team:SIEM v7.9.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants