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

[Test] Enable unit test suite: get_local_stats.test.ts #528

Merged
merged 2 commits into from
Jun 26, 2021

Conversation

ananzh
Copy link
Member

@ananzh ananzh commented Jun 22, 2021

Description

All the unit tests related to unused telemetry are temporarily
skipped after the fork. Unit tests of the disabled telemetry
functions should also be modified correspondingly. To build
a clean unit test, we decide to modify and enable all the
working unit tests. This PR checks and enables
get_local_stats.test.ts.

Signed-off-by: Anan Zhuang [email protected]

Issues Resolved

#512

Test results

unit test for get_local_stats.test.ts

yarn test:jest /home/anan/work/OpenSearch-Dashboards/src/plugins/telemetry/server/telemetry_collection/get_local_stats.test.ts
yarn run v1.22.10
$ node scripts/jest /home/anan/work/OpenSearch-Dashboards/src/plugins/telemetry/server/telemetry_collection/get_local_stats.test.ts
 PASS  src/plugins/telemetry/server/telemetry_collection/get_local_stats.test.ts
  get_local_stats
    handleLocalStats
      ✓ returns expected object without OpenSearch Dashboards data (21 ms)
      ✓ returns expected object with xpack (6 ms)
    getLocalStats
      ✓ returns expected object with OpenSearch Dashboards data (30 ms)
      ✓ returns an empty array when no cluster uuid is provided (15 ms)

  console.warn
    No OpenSearchDashboards stats returned from usage collectors

      60 | ) {
      61 |   if (!response) {
    > 62 |     logger.warn('No OpenSearchDashboards stats returned from usage collectors');
         |            ^
      63 |     return;
      64 |   }
      65 |   const {

      at handleOpenSearchDashboardsStats (src/plugins/telemetry/server/telemetry_collection/get_opensearch_dashboards.ts:62:12)
      at handleLocalStats (src/plugins/telemetry/server/telemetry_collection/get_local_stats.ts:73:30)
      at Object.it (src/plugins/telemetry/server/telemetry_collection/get_local_stats.test.ts:199:22)

  console.warn
    No OpenSearchDashboards stats returned from usage collectors

      60 | ) {
      61 |   if (!response) {
    > 62 |     logger.warn('No OpenSearchDashboards stats returned from usage collectors');
         |            ^
      63 |     return;
      64 |   }
      65 |   const {

      at handleOpenSearchDashboardsStats (src/plugins/telemetry/server/telemetry_collection/get_opensearch_dashboards.ts:62:12)
      at handleLocalStats (src/plugins/telemetry/server/telemetry_collection/get_local_stats.ts:73:30)
      at Object.it (src/plugins/telemetry/server/telemetry_collection/get_local_stats.test.ts:216:22)

Test Suites: 1 passed, 1 total
Tests:       4 passed, 4 total
Snapshots:   0 total
Time:        2.618 s

Overall test result:
Screen Shot 2021-06-21 at 9 42 20 PM

Check List

  • New functionality includes testing.
    • All tests pass
  • New functionality has been documented.
    • New functionality has javadoc added
  • Commits are signed per the DCO using --signoff

…oject#512)

All the unit tests related to unused telemetry are temporarily
skipped after the fork. Unit tests of the disabled telemetry
functions should also be modified correspondingly. To build
a clean unit test, we decide to modify and enable all the
working unit tests. This PR checks and enables
get_local_stats.test.ts.

Signed-off-by: Anan Zhuang <[email protected]>
@opensearch-ci-bot
Copy link
Collaborator

✅   DCO Check Passed 5a9b507

tmarkley
tmarkley previously approved these changes Jun 22, 2021
@ananzh ananzh mentioned this pull request Jun 23, 2021
26 tasks
@kavilla kavilla linked an issue Jun 23, 2021 that may be closed by this pull request
@kavilla
Copy link
Member

kavilla commented Jun 23, 2021

✓ returns expected object with xpack (6 ms)

We can probably delete this test. Also I see the warning:

  No OpenSearchDashboards stats returned from usage collectors

Local to me implies no external dependencies. Should this logging this warning message? Not so much related to this PR but we might have broken basic (free) telemetry that is stored in the engine. If it was still suppose to depend on a third party then should we update the test to expect a warning for now that way we know we correctly implemented as expected in the current code base when the warning is no longer being thrown.

Copy link
Member

@kavilla kavilla left a comment

Choose a reason for hiding this comment

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

Mostly looks good. Just a few comments.

Remove test case: returns expected object with xpack
This is a unit test case related to xpack which does
not exist in dashboards code.

Signed-off-by: Anan Zhuang <[email protected]>
@opensearch-ci-bot
Copy link
Collaborator

✅   DCO Check Passed c16db36

@ananzh
Copy link
Member Author

ananzh commented Jun 25, 2021

✓ returns expected object with xpack (6 ms)

We can probably delete this test. Also I see the warning:

  No OpenSearchDashboards stats returned from usage collectors

Local to me implies no external dependencies. Should this logging this warning message? Not so much related to this PR but we might have broken basic (free) telemetry that is stored in the engine. If it was still suppose to depend on a third party then should we update the test to expect a warning for now that way we know we correctly implemented as expected in the current code base when the warning is no longer being thrown.

I removed the xpack test. Thanks for pointing it out. For the warning message, I will open an issue later and will do more investigation.

@ananzh
Copy link
Member Author

ananzh commented Jun 25, 2021

Follow up issue: #556

@kavilla kavilla self-requested a review June 25, 2021 23:19
@ananzh ananzh merged commit 4dcf30b into opensearch-project:main Jun 26, 2021
kavilla pushed a commit that referenced this pull request Jun 26, 2021
* [Test] Enable unit test suite: get_local_stats.test.ts (#512)

All the unit tests related to unused telemetry are temporarily
skipped after the fork. Unit tests of the disabled telemetry
functions should also be modified correspondingly. To build
a clean unit test, we decide to modify and enable all the
working unit tests. This PR checks and enables
get_local_stats.test.ts.

Signed-off-by: Anan Zhuang <[email protected]>

* [Test] Remove unit test case related to xpack (#512)

Remove test case: returns expected object with xpack
This is a unit test case related to xpack which does
not exist in dashboards code.

Signed-off-by: Anan Zhuang <[email protected]>
kavilla pushed a commit that referenced this pull request Jun 26, 2021
* [Test] Enable unit test suite: get_local_stats.test.ts (#512)

All the unit tests related to unused telemetry are temporarily
skipped after the fork. Unit tests of the disabled telemetry
functions should also be modified correspondingly. To build
a clean unit test, we decide to modify and enable all the
working unit tests. This PR checks and enables
get_local_stats.test.ts.

Signed-off-by: Anan Zhuang <[email protected]>

* [Test] Remove unit test case related to xpack (#512)

Remove test case: returns expected object with xpack
This is a unit test case related to xpack which does
not exist in dashboards code.

Signed-off-by: Anan Zhuang <[email protected]>
@ananzh ananzh deleted the i-512 branch February 23, 2022 17:11
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.

[Test] unit test get_local_stats.test is skipped now
4 participants