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

tbi(telemetry): update init settings, bump azure applicationinsights and stablise tests #1714

Merged
merged 5 commits into from
Mar 4, 2024

Conversation

cianmSAP
Copy link
Contributor

@cianmSAP cianmSAP commented Mar 1, 2024

#1417

  • Adds watchTelemetrySettingStore as a required option to ToolsSuiteTelemetryInitSettings. Using a watch on the store is not always desirable

  • Bump applicationinsights to 2.9.2 to incorporate fix Minimal fix for #1226 microsoft/ApplicationInsights-node.js#1241 displaying ApplicationInsights:Invalid JSON config file when using APPLICATIONINSIGHTS_CONNECTION_STRING error

  • Increasing timeout for tests due to instabilities. Also making paths dynamic so they can be ran with tools like jest-runner

Copy link

changeset-bot bot commented Mar 1, 2024

🦋 Changeset detected

Latest commit: 130addf

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@sap-ux/telemetry Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@tobiasqueck
Copy link
Contributor

@cianmSAP do you know a little more about the instabilities? What is taking so long?

IainSAP
IainSAP previously approved these changes Mar 1, 2024
Copy link
Contributor

@IainSAP IainSAP left a comment

Choose a reason for hiding this comment

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

Thanks @cianmSAP

  • Timeout inc. to avoid timeout on long running tests.
  • Nothing to be tested

@cianmSAP
Copy link
Contributor Author

cianmSAP commented Mar 1, 2024

@cianmSAP do you know a little more about the instabilities? What is taking so long?

After some investigation I can see that getAppProperties

async function getAppProperties(appPath: string): Promise<Record<string, string>> {

can take a 3-4 seconds to complete in some cases (atleast on my machine). I think the timeout here covers this unpredictability

@tobiasqueck

@cianmSAP cianmSAP changed the title chore(telemetry): fix unstable unit tests chore(telemetry): fix unstable unit tests & bump applicationinsights version Mar 1, 2024
@cianmSAP cianmSAP changed the title chore(telemetry): fix unstable unit tests & bump applicationinsights version chore(telemetry): update init settings, bump azure applicationinsights and stablise tests Mar 4, 2024
@cianmSAP cianmSAP changed the title chore(telemetry): update init settings, bump azure applicationinsights and stablise tests tbi(telemetry): update init settings, bump azure applicationinsights and stablise tests Mar 4, 2024
@cianmSAP cianmSAP requested review from IainSAP and devinea March 4, 2024 12:44
Copy link
Member

@devinea devinea left a comment

Choose a reason for hiding this comment

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

Thanks for the update.
updates are clear and covered by tests.
changeset ✅

Copy link
Contributor

@IainSAP IainSAP left a comment

Choose a reason for hiding this comment

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

Re-approving

Copy link

sonarqubecloud bot commented Mar 4, 2024

@cianmSAP cianmSAP merged commit 15d6afb into main Mar 4, 2024
13 checks passed
@cianmSAP cianmSAP deleted the chore/telemetry/fix-unit-tests branch March 4, 2024 15:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants