Skip to content

Conversation

junaed-optimizely
Copy link
Contributor

@junaed-optimizely junaed-optimizely commented Sep 2, 2024

Summary

Significant improvements were made to the test coverage of the "Client" and "Provider files.

  • Provider Coverage: Increased from 72.34% to 100%
  • Client Coverage: Improved from 81.4% to 99.27% by restructuring and adding tests for various methods
  • Test Additions: A total of 52 new tests were added, covering previously missing cases and deprecated API updates.

The coverage reports have been generated with the help of "Istanbul" that is the default coverage tool of "Jest". Here are the commands that I have used to generate the coverage report -

# for client
npx jest --coverage -- client.spec.ts
# and for provider
npx jest --coverage -- provider.spec.tsx

Test plan

A total of 52 new tests were added, covering previously missing cases and deprecated API updates.

Issues

@junaed-optimizely junaed-optimizely marked this pull request as ready for review September 2, 2024 14:46
@junaed-optimizely junaed-optimizely changed the title Junaed/fssdk 10439 client provider test [FSSDK 10439] client provider test Sep 2, 2024
@junaed-optimizely junaed-optimizely changed the title [FSSDK 10439] client provider test [FSSDK-10439] client provider test Sep 2, 2024
Copy link
Contributor

@mikechu-optimizely mikechu-optimizely left a comment

Choose a reason for hiding this comment

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

What are your thoughts on splitting out client.spec.ts into separate files? Along with less cognitive load for review/maintenance, I think we get some speed of execution benefit too. 🤔

Otherwise, LGTM

@mikechu-optimizely
Copy link
Contributor

NIT: I like a bit of whitespace between test blocks, and even to separate the arrange-act-assert within each test. imho

@junaed-optimizely
Copy link
Contributor Author

NIT: I like a bit of whitespace between test blocks, and even to separate the arrange-act-assert within each test. imho

Thats a good suggestion. Let me do that

@junaed-optimizely
Copy link
Contributor Author

junaed-optimizely commented Sep 3, 2024

What are your thoughts on splitting out client.spec.ts into separate files? Along with less cognitive load for review/maintenance, I think we get some speed of execution benefit too. 🤔

Otherwise, LGTM

I like your suggestion. But its not uncommon to have a single test file against a single module. It offers easier context management as well.

@junaed-optimizely junaed-optimizely merged commit 62a12ff into master Sep 4, 2024
9 checks passed
@junaed-optimizely junaed-optimizely deleted the junaed/fssdk-10439-client-provider-test branch September 4, 2024 13:29
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.

2 participants