Skip to content

Unit Tests for common/lib #53736

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 6 commits into from
Jan 10, 2020
Merged

Unit Tests for common/lib #53736

merged 6 commits into from
Jan 10, 2020

Conversation

@timductive
Copy link
Member

@timductive timductive commented Dec 20, 2019 •
edited
Loading

Summary

Adding jest tests for common/lib directory.

Closes #23076

CODE COVERAGE BEFORE:
Screen Shot 2019-12-20 at 4 26 30 PM

CODE COVERAGE AFTER:
Screen Shot 2020-01-03 at 4 11 35 PM

Checklist

- [ ] This was checked for cross-browser compatibility, including a check against IE11
- [ ] Any text added follows EUI's writing guidelines, uses sentence case text and includes i18n support
- [ ] Documentation was added for features that require explanation or tutorials

For maintainers

@timductive timductive added Team:Presentation Presentation Team for Dashboard, Input Controls, and Canvas t// release_note:skip Skip the PR/issue when compiling release notes v7.6.0 labels Dec 20, 2019
@timductive timductive self-assigned this Dec 20, 2019
@elasticmachine
Copy link
Contributor

elasticmachine commented Dec 20, 2019

Pinging @elastic/kibana-canvas (Team:Canvas)

timductive
timductive commented Dec 20, 2019
View reviewed changes
x-pack/legacy/plugins/canvas/common/lib/__tests__/errors.test.js Outdated
Copy link
Member Author

@timductive timductive Dec 20, 2019

Choose a reason for hiding this comment

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

I'm sure there's a better way to do this...

@elasticmachine
Copy link
Contributor

elasticmachine commented Dec 20, 2019

💔 Build Failed

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

@timductive timductive force-pushed the canvas-common-tests branch from 31adcf9 to a4839d8 Compare January 3, 2020 19:45
@timductive timductive marked this pull request as ready for review January 3, 2020 19:46
@timductive timductive requested a review from a team as a code owner January 3, 2020 19:46
@timductive
Copy link
Member Author

timductive commented Jan 8, 2020

@elasticmachine merge upstream

poffdeluxe
poffdeluxe approved these changes Jan 8, 2020
View reviewed changes
Copy link
Contributor

@poffdeluxe poffdeluxe left a comment

Choose a reason for hiding this comment

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

Overall looks good, just a couple of comments re:test result descriptions

x-pack/legacy/plugins/canvas/common/lib/hex_to_rgb.test.ts Outdated
Copy link
Contributor

@poffdeluxe poffdeluxe Jan 8, 2020

Choose a reason for hiding this comment

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

minor but if we're switching to use the it syntax, I think we should make the descriptions match that. For example, changing invalid hex here to should return null for invalid hex values

x-pack/legacy/plugins/canvas/common/lib/dataurl.test.ts Outdated
Copy link
Contributor

@poffdeluxe poffdeluxe Jan 8, 2020

Choose a reason for hiding this comment

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

same comment as the hex tests

crob611
crob611 approved these changes Jan 8, 2020
View reviewed changes
Copy link
Contributor

@crob611 crob611 left a comment

Choose a reason for hiding this comment

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

Looks great. Nice work!

x-pack/legacy/plugins/canvas/common/lib/get_field_type.test.ts Outdated
Copy link
Contributor

@crob611 crob611 Jan 8, 2020

Choose a reason for hiding this comment

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

This path is weird with a leading single dot. Can you just do ../../ instead?

@kibanamachine
Copy link
Contributor

kibanamachine commented Jan 10, 2020

💚 Build Succeeded

History

  • 💚 Build #18703 succeeded 8b72aae302452fce65095d4768a387bd92ce95f4
  • 💚 Build #17898 succeeded 944705681b593957f22f2f49a3c0d7f62bc927df
  • 💔 Build #17865 failed a4839d89ee4c4cee5e6baabf313b594e251917f2
  • 💔 Build #17045 failed 31adcf981f8880d2b678d23dc1d3b06c17661227

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

@timductive timductive merged commit c9e4304 into elastic:master Jan 10, 2020
@timductive timductive deleted the canvas-common-tests branch January 10, 2020 20:02
timductive added a commit to timductive/kibana that referenced this pull request Jan 10, 2020
@timductive
Unit Tests for common/lib (elastic#53736)
b16252f 
* converting mocha tests to jest

* adding a few lib tests

* adding more lib tests

* moving test files and adding autocomplete tests

* updating test definition

* fixing import and test definitions
@timductive timductive mentioned this pull request Jan 10, 2020
Merged
timductive added a commit that referenced this pull request Jan 11, 2020
@timductive @elasticmachine
Unit Tests for common/lib (#53736) (#54496)
050e797 
* converting mocha tests to jest

* adding a few lib tests

* adding more lib tests

* moving test files and adding autocomplete tests

* updating test definition

* fixing import and test definitions

Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
gmmorris added a commit to gmmorris/kibana that referenced this pull request Jan 13, 2020
@gmmorris
Merge branch 'master' into task-manager/schema-config
4173614 
* master: (69 commits)
  [Graph] Fix various a11y issues (elastic#54097)
  Add ApplicationService app status management (elastic#50223)
  logs in one time (elastic#54447)
  Deprecate using `elasticsearch.ssl.certificate` without `elasticsearch.ssl.key` and vice versa (elastic#54392)
  [Optimizer] Fix a stack overflow with watch_cache when it attempts to delete very large folders. (elastic#54457)
  Security - Role Mappings UI (elastic#53620)
  [SIEM] [Detection engine] Permission II (elastic#54292)
  Allow User to Cleanup Repository from UI  (elastic#53047)
  [Detection engine] Some UX for rule creation (elastic#54471)
  share specific instances of some ui packages (elastic#54079)
  [ML] APM modules configs for RUM Javascript and NodeJS (elastic#53792)
  [APM] Delay rendering invalid license notification (elastic#53924)
  [Graph] Improve error message on graph requests (elastic#54230)
  [ILM] Kibana should allow a min_age setting of 0ms in ILM policy phases (elastic#53719)
  Unit Tests for common/lib (elastic#53736)
  [Graph] Only show explorable fields (elastic#54101)
  remove linting rule exception for markdown (elastic#54232)
  [Monitoring] Fetch shard data more efficiently (elastic#54028)
  [Maps] Add hiddenLayers option to embeddable map input (elastic#54355)
  Pass termOrder and hasTermsAgg properties to serializeThresholdWatch function (elastic#54391)
  ...
gmmorris added a commit to gmmorris/kibana that referenced this pull request Jan 13, 2020
@gmmorris
Merge branch 'master' into alerting/zombie-redux
0ce6ed5 
* master: (69 commits)
  [Graph] Fix various a11y issues (elastic#54097)
  Add ApplicationService app status management (elastic#50223)
  logs in one time (elastic#54447)
  Deprecate using `elasticsearch.ssl.certificate` without `elasticsearch.ssl.key` and vice versa (elastic#54392)
  [Optimizer] Fix a stack overflow with watch_cache when it attempts to delete very large folders. (elastic#54457)
  Security - Role Mappings UI (elastic#53620)
  [SIEM] [Detection engine] Permission II (elastic#54292)
  Allow User to Cleanup Repository from UI  (elastic#53047)
  [Detection engine] Some UX for rule creation (elastic#54471)
  share specific instances of some ui packages (elastic#54079)
  [ML] APM modules configs for RUM Javascript and NodeJS (elastic#53792)
  [APM] Delay rendering invalid license notification (elastic#53924)
  [Graph] Improve error message on graph requests (elastic#54230)
  [ILM] Kibana should allow a min_age setting of 0ms in ILM policy phases (elastic#53719)
  Unit Tests for common/lib (elastic#53736)
  [Graph] Only show explorable fields (elastic#54101)
  remove linting rule exception for markdown (elastic#54232)
  [Monitoring] Fetch shard data more efficiently (elastic#54028)
  [Maps] Add hiddenLayers option to embeddable map input (elastic#54355)
  Pass termOrder and hasTermsAgg properties to serializeThresholdWatch function (elastic#54391)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

2 more reviewers

@crob611 crob611 crob611 approved these changes

@poffdeluxe poffdeluxe poffdeluxe approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@timductive timductive

Labels

release_note:skip Skip the PR/issue when compiling release notes review Team:Presentation Presentation Team for Dashboard, Input Controls, and Canvas t// v7.6.0

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Unit Tests for Common Functions: Lib

5 participants

@timductive @elasticmachine @kibanamachine @crob611 @poffdeluxe