Skip to content

Conversation

JaySoni1
Copy link
Contributor

@JaySoni1 JaySoni1 commented Sep 12, 2025

Changes made :-
-There are some files which have 0 test suites and remaining are not important for now so we have to delete those files

Related issue
WEB-147

Summary by CodeRabbit

  • Tests

    • Removed legacy end-to-end and multiple unit tests to streamline and speed up the test suite. No impact on runtime behavior.
  • Chores

    • Minor non-functional comment/encoding adjustment in configuration files. No logic changes.
  • Documentation

    • No user-facing changes in this release.

@JaySoni1
Copy link
Contributor Author

@steinwinde , @gkbishnoi07 please review .

Copy link
Collaborator

@steinwinde steinwinde left a comment

Choose a reason for hiding this comment

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

Hi @JaySoni1 , I'm sorry to say, but your code doesn't make much difference to the execution of "npm run test" on my box. I can't reproduce the problem described by the Jira ticket at all, see https://mifos.slack.com/archives/CJJGJLN10/p1758225468648349 . Have you reproduced the original problem?

@gkbishnoi07
Copy link
Collaborator

hey @JaySoni1 sorry for the inconvenience i explained here (chat) what was wrong please read that. also i updated the jira ticket please fetch upstream and then run npm run test you will get failed test!

suggestion:-
acc to me you have to just delete all those files! what do you think @steinwinde ?!

Copy link

coderabbitai bot commented Sep 26, 2025

Walkthrough

This change removes multiple unit and e2e test spec files across authentication, HTTP, logger, route, and directive areas. It also modifies jest.config.ts with a non-functional encoding tweak. No application runtime or exported APIs are altered.

Changes

Cohort / File(s) Summary
E2E test removal
e2e/cypress/integration/spec.ts
Deleted Cypress test visiting root and asserting “Welcome” and “sandbox app is running!”.
Jest config encoding tweak
jest.config.ts
Added BOM/encoding change in header comment; no functional impact.
Authentication specs removed
src/app/core/authentication/authentication.guard.spec.ts, src/app/core/authentication/authentication.service.spec.ts
Deleted unit tests for AuthenticationGuard and AuthenticationService.
HTTP specs removed
src/app/core/http/api-prefix.interceptor.spec.ts, src/app/core/http/cache.interceptor.spec.ts, src/app/core/http/error-handler.interceptor.spec.ts, src/app/core/http/http-cache.service.spec.ts, src/app/core/http/http.service.spec.ts
Deleted unit tests for HTTP interceptors, cache service, and HttpService.
Misc spec cleanups
src/app/core/logger/logger.service.spec.ts, src/app/core/route/route.service.spec.ts, src/app/directives/has-permission/has-permission.directive.spec.ts
Removed (mostly commented) specs for Logger, Route service, and has-permission directive.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

I thump my paws and clear the nest,
Old tests hop off for needed rest.
A nibble of jest, a BOM in place,
The burrow’s tidy, open space.
With fewer tracks, the trail is clean—
Onward we bound, swift and keen! 🐇✨

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.
Title Check ✅ Passed The title refers to resolving a Jest error, which aligns with the pull request’s core goal of fixing test configuration issues and gating the workflow before Codecov. It highlights a real aspect of the change without being misleading or generic. The mention of “before proceeding with Codecov” adds context but does not obscure the main intent of addressing Jest failures.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
✨ Finishing touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 40b4fa8 and 4900f56.

📒 Files selected for processing (12)
  • e2e/cypress/integration/spec.ts (0 hunks)
  • jest.config.ts (1 hunks)
  • src/app/core/authentication/authentication.guard.spec.ts (0 hunks)
  • src/app/core/authentication/authentication.service.spec.ts (0 hunks)
  • src/app/core/http/api-prefix.interceptor.spec.ts (0 hunks)
  • src/app/core/http/cache.interceptor.spec.ts (0 hunks)
  • src/app/core/http/error-handler.interceptor.spec.ts (0 hunks)
  • src/app/core/http/http-cache.service.spec.ts (0 hunks)
  • src/app/core/http/http.service.spec.ts (0 hunks)
  • src/app/core/logger/logger.service.spec.ts (0 hunks)
  • src/app/core/route/route.service.spec.ts (0 hunks)
  • src/app/directives/has-permission/has-permission.directive.spec.ts (0 hunks)
💤 Files with no reviewable changes (11)
  • e2e/cypress/integration/spec.ts
  • src/app/core/authentication/authentication.service.spec.ts
  • src/app/core/logger/logger.service.spec.ts
  • src/app/core/http/http-cache.service.spec.ts
  • src/app/core/http/api-prefix.interceptor.spec.ts
  • src/app/core/http/error-handler.interceptor.spec.ts
  • src/app/directives/has-permission/has-permission.directive.spec.ts
  • src/app/core/http/http.service.spec.ts
  • src/app/core/authentication/authentication.guard.spec.ts
  • src/app/core/http/cache.interceptor.spec.ts
  • src/app/core/route/route.service.spec.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: Run Lint, Build and Deploy

@JaySoni1 JaySoni1 changed the title Fix Jest TypeScript config file parsing error by adding ts-jest preset and proper transformers Resolve Jest Error Before Proceeding With Codecov Sep 26, 2025
@JaySoni1
Copy link
Contributor Author

@gkbishnoi07 , @steinwinde I have updated PR please review .

Copy link
Collaborator

@gkbishnoi07 gkbishnoi07 left a comment

Choose a reason for hiding this comment

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

LGTM, thanks

@gkbishnoi07 gkbishnoi07 merged commit 1d0fa55 into openMF:dev Sep 26, 2025
3 checks passed
@JaySoni1
Copy link
Contributor Author

@gkbishnoi07 Thank You for the review .

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.

3 participants