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: telemetry_service.test.ts #502

Merged
merged 1 commit into from
Jun 26, 2021

Conversation

ananzh
Copy link
Member

@ananzh ananzh commented Jun 18, 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 modifies and enables
telemetry_service.test.ts

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

Issues Resolved

#499 (1 test case is skipped)

Test results

unit test for telemetry_service.test.ts

yarn test:jest /home/anan/work/OpenSearch-Dashboards/src/plugins/telemetry/public/services/telemetry_service.test.ts
yarn run v1.22.5
$ node scripts/jest /home/anan/work/OpenSearch-Dashboards/src/plugins/telemetry/public/services/telemetry_service.test.ts
 PASS  src/plugins/telemetry/public/services/telemetry_service.test.ts
  TelemetryService
    fetchTelemetry
      ○ skipped calls expected URL with 20 minutes - now
    fetchExample
      ✓ calls fetchTelemetry with unencrupted: true (4 ms)
    setOptIn
      ✓ does not call the api if canChangeOptInStatus==false (1 ms)
      ✓ calls api if canChangeOptInStatus
      ✓ sends enabled true if optedIn: true (1 ms)
      ✓ sends enabled false if optedIn: false (1 ms)
      ✓ does not call reportOptInStatus if reportOptInStatusChange is false
      ✓ calls reportOptInStatus if reportOptInStatusChange is true (70 ms)
      ✓ adds an error toast on api error (2 ms)
      ✓ adds an error toast on reportOptInStatus error (1 ms)
    getTelemetryUrl
      ✓ should return the config.url parameter
    setUserHasSeenNotice
      ✓ should hit the API and change the config (1 ms)
      ✓ should show a toast notification if the request fail (1 ms)
    getUserShouldSeeOptInNotice
      ✓ returns whether the user can update the telemetry config (has SavedObjects access) (2 ms)

  console.error
    Error: Error: connect ECONNREFUSED 127.0.0.1:80
        at Object.dispatchError (/home/anan/work/OpenSearch-Dashboards/node_modules/jsdom/lib/jsdom/living/xhr-utils.js:60:19)
        at Request.client.on.err (/home/anan/work/OpenSearch-Dashboards/node_modules/jsdom/lib/jsdom/living/xmlhttprequest.js:674:20)
        at Request.emit (events.js:203:15)
        at Request.onRequestError (/home/anan/work/OpenSearch-Dashboards/node_modules/request/request.js:877:8)
        at ClientRequest.emit (events.js:198:13)
        at Socket.socketErrorListener (_http_client.js:401:9)
        at Socket.emit (events.js:198:13)
        at emitErrorNT (internal/streams/destroy.js:91:8)
        at emitErrorAndCloseNT (internal/streams/destroy.js:59:3)
        at process._tickCallback (internal/process/next_tick.js:63:19) undefined

      at VirtualConsole.on.e (node_modules/jsdom/lib/jsdom/virtual-console.js:29:45)
      at Object.dispatchError (node_modules/jsdom/lib/jsdom/living/xhr-utils.js:63:53)
      at Request.client.on.err (node_modules/jsdom/lib/jsdom/living/xmlhttprequest.js:674:20)
      at Request.onRequestError (node_modules/request/request.js:877:8)

Test Suites: 1 passed, 1 total
Tests:       1 skipped, 13 passed, 14 total
Snapshots:   0 total
Time:        3.627 s

Overall test result:
Screen Shot 2021-06-18 at 4 01 45 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

@opensearch-ci-bot
Copy link
Collaborator

✅   DCO Check Passed 7971c22

@ananzh ananzh mentioned this pull request Jun 18, 2021
26 tasks
@ananzh ananzh changed the title [Test] Enable unit test suite: telemetry_service.test.ts (#499) [Test] Enable unit test suite: telemetry_service.test.ts Jun 18, 2021
@kavilla
Copy link
Member

kavilla commented Jun 23, 2021

You commented:

console.error
    Error: Error: connect ECONNREFUSED 127.0.0.1:80
        at Object.dispatchError (/home/anan/work/OpenSearch-Dashboards/node_modules/jsdom/lib/jsdom/living/xhr-utils.js:60:19)
        at Request.client.on.err (/home/anan/work/OpenSearch-Dashboards/node_modules/jsdom/lib/jsdom/living/xmlhttprequest.js:674:20)
        at Request.emit (events.js:203:15)
        at Request.onRequestError (/home/anan/work/OpenSearch-Dashboards/node_modules/request/request.js:877:8)
        at ClientRequest.emit (events.js:198:13)
        at Socket.socketErrorListener (_http_client.js:401:9)
        at Socket.emit (events.js:198:13)
        at emitErrorNT (internal/streams/destroy.js:91:8)
        at emitErrorAndCloseNT (internal/streams/destroy.js:59:3)
        at process._tickCallback (internal/process/next_tick.js:63:19) undefined

      at VirtualConsole.on.e (node_modules/jsdom/lib/jsdom/virtual-console.js:29:45)
      at Object.dispatchError (node_modules/jsdom/lib/jsdom/living/xhr-utils.js:63:53)
      at Request.client.on.err (node_modules/jsdom/lib/jsdom/living/xmlhttprequest.js:674:20)
      at Request.onRequestError (node_modules/request/request.js:877:8)

Any idea if this can be addressed?

…project#499)

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 modifies and enables
telemetry_service.test.ts.

There is one test case left: calls expected URL with 20 minutes
This test case is skipped due to function fetchTelemetry is disabled.
We should enable this test when fetchTelemtry function is restored.

Partially solved issue:
opensearch-project#499

Signed-off-by: Anan Zhuang <[email protected]>
@ananzh ananzh reopened this Jun 25, 2021
@opensearch-ci-bot
Copy link
Collaborator

✅   DCO Check Passed 0b21562

Copy link
Member Author

@ananzh ananzh left a comment

Choose a reason for hiding this comment

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

Resubmit PR. Please take another look @tmarkley @kavilla

@ananzh
Copy link
Member Author

ananzh commented Jun 25, 2021

You commented:

console.error
    Error: Error: connect ECONNREFUSED 127.0.0.1:80
        at Object.dispatchError (/home/anan/work/OpenSearch-Dashboards/node_modules/jsdom/lib/jsdom/living/xhr-utils.js:60:19)
        at Request.client.on.err (/home/anan/work/OpenSearch-Dashboards/node_modules/jsdom/lib/jsdom/living/xmlhttprequest.js:674:20)
        at Request.emit (events.js:203:15)
        at Request.onRequestError (/home/anan/work/OpenSearch-Dashboards/node_modules/request/request.js:877:8)
        at ClientRequest.emit (events.js:198:13)
        at Socket.socketErrorListener (_http_client.js:401:9)
        at Socket.emit (events.js:198:13)
        at emitErrorNT (internal/streams/destroy.js:91:8)
        at emitErrorAndCloseNT (internal/streams/destroy.js:59:3)
        at process._tickCallback (internal/process/next_tick.js:63:19) undefined

      at VirtualConsole.on.e (node_modules/jsdom/lib/jsdom/virtual-console.js:29:45)
      at Object.dispatchError (node_modules/jsdom/lib/jsdom/living/xhr-utils.js:63:53)
      at Request.client.on.err (node_modules/jsdom/lib/jsdom/living/xmlhttprequest.js:674:20)
      at Request.onRequestError (node_modules/request/request.js:877:8)

Any idea if this can be addressed?

I think this error message is expected. But I will open an issue later and will do more investigation.

@tmarkley
Copy link
Contributor

I think this error message is expected. But I will open an issue later and will do more investigation.

@ananzh can you open the issue now so that we have it documented?

@ananzh
Copy link
Member Author

ananzh commented Jun 25, 2021

I think this error message is expected. But I will open an issue later and will do more investigation.

@ananzh can you open the issue now so that we have it documented?

@tmarkley yes, this is the follow up issue: #557

@ananzh ananzh merged commit 061feb9 into opensearch-project:main Jun 26, 2021
kavilla pushed a commit that referenced this pull request Jun 26, 2021
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 modifies and enables
telemetry_service.test.ts.

There is one test case left: calls expected URL with 20 minutes
This test case is skipped due to function fetchTelemetry is disabled.
We should enable this test when fetchTelemtry function is restored.

Partially solved issue:
#499

Signed-off-by: Anan Zhuang <[email protected]>
kavilla pushed a commit that referenced this pull request Jun 26, 2021
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 modifies and enables
telemetry_service.test.ts.

There is one test case left: calls expected URL with 20 minutes
This test case is skipped due to function fetchTelemetry is disabled.
We should enable this test when fetchTelemtry function is restored.

Partially solved issue:
#499

Signed-off-by: Anan Zhuang <[email protected]>
@ananzh ananzh deleted the i-499 branch September 14, 2021 17:34
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.

4 participants