-
-
Notifications
You must be signed in to change notification settings - Fork 213
Bugfix: logger LWC Initialization Message #884
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
Conversation
…sage of 'logger component initialized' when console logging is enabled via LoggerSettings__c.IsJavaScriptConsoleLoggingEnabled__c
…eed for enableSystemMessages() and disableSystemMessages() functions in loggerService.js - Replaced enableSystemMessages() and disableSystemMessages() with new static variable LoggerService.areSystemMessagesEnabled - Switched to using jest.resetAllMocks() instead of jest.clearAllMocks() for better test isolation - Updated tests to reset LoggerService.hasInitialized and LoggerService.areSystemMessagesEnabled static variables in beforeEach() instead of afterAll() - Updated all test functions to consistently use all logger creation methods (createLogger, getLogger, getMarkupLogger)
04155b3 to
f3339bc
Compare
…nsate for the lower code coverage in LWC tests for now Eventually, code coverage for LWC will be increased - but for now, the constant alerts from codecov.io aren't particularly helpful ^_^
f3339bc to
d19ee0b
Compare
| import { BrowserContext } from '../loggerService'; | ||
| import LoggerService from '../loggerService'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jamessimone random readability/style-preference question for you - for imports like this (with a default + named exports), do you go for a one-liner? Or do you like to keep them separated?
| import { BrowserContext } from '../loggerService'; | |
| import LoggerService from '../loggerService'; | |
| import LoggerService, { BrowserContext } from '../loggerService'; |
8624df3 to
3296b6b
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #884 +/- ##
==========================================
+ Coverage 86.19% 91.50% +5.31%
==========================================
Files 17 58 +41
Lines 1318 6312 +4994
Branches 199 199
==========================================
+ Hits 1136 5776 +4640
- Misses 162 516 +354
Partials 20 20
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Fixed #870 by updating the
loggerLWC initialization message to only print when JavaScript console logging is enabled viaLoggerSettings__c. Also resolved some test isolation issues inlogger.test.js+ test improvements + code cleanuploggerService.jsto only callconsole.info()with an init message ('logger component initialized') when console logging is enabled viaLoggerSettings__c.IsJavaScriptConsoleLoggingEnabled__cenableSystemMessages()anddisableSystemMessages()inloggerService.jswith a new static variableLoggerService.areSystemMessagesEnabled(intended for internal-use only)logger.test.jsto resetLoggerService.hasInitializedandLoggerService.areSystemMessagesEnabledstatic variables inbeforeEach()(instead ofafterAll())jest.resetAllMocks()instead ofjest.clearAllMocks()inlogger.test.jsfor better test isolationcreateLogger,getLogger, andgetMarkupLogger)Also include a 🤏 liiiiiiitle bit of scope creep:
codecov.ymlto compensate for the lower code coverage in LWC tests for now (currently around ~86%). Eventually, tests & code coverage for LWC will be improved - but for now, the constant pipeline alerts from codecov.io aren't particularly helpful 😅LogBatchPurgerusing the null coalescing operator (??) to reduce the lines of code neededLoggerScenarioRule__mdt.IsLogAssignmentEnabled__cto use the termLogger Scenario, instead of the deprecated termLog Scenario