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.

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

Unit Tests for common/lib #53736

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 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

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

describe('errors', () => {
it('creates a test error', () => {
// eslint-disable-next-line new-cap
Copy link
Member Author

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

💔 Build Failed

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

@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

@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

describe('hexToRgb', () => {
test('invalid hex', () => {
it('invalid hex', () => {
Copy link
Contributor

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
@@ -34,10 +39,13 @@ describe('dataurl', () => {
});

describe('dataurl.parseDataUrl', () => {
test('invalid data url', () => {
it('invalid data url', () => {
Copy link
Contributor

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
import {
emptyTable,
testTable,
} from '../../../canvas_plugin_src/functions/common/__tests__/fixtures/test_tables';
} from './../../canvas_plugin_src/functions/common/__tests__/fixtures/test_tables';
Copy link
Contributor

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

💚 Build Succeeded

History

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>
thomasneirynck pushed a commit to thomasneirynck/kibana that referenced this pull request Jan 12, 2020
@timductive @thomasneirynck
Unit Tests for common/lib (elastic#53736)
b2d31e2 
* 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
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

@crob611 crob611 crob611 approved these changes

@poffdeluxe poffdeluxe poffdeluxe approved these changes

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 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