diff --git a/src/renderer/__mocks__/notifications-mocks.ts b/src/renderer/__mocks__/notifications-mocks.ts index 1bcae1fc1..d5522fa6a 100644 --- a/src/renderer/__mocks__/notifications-mocks.ts +++ b/src/renderer/__mocks__/notifications-mocks.ts @@ -3,6 +3,7 @@ import type { StateType, Subject, SubjectType } from '../typesGitHub'; import { mockEnterpriseNotifications, mockGitHubNotifications, + mockSingleNotification, } from '../utils/api/__mocks__/response-mocks'; import { mockGitHubCloudAccount, @@ -25,7 +26,7 @@ export const mockAccountNotifications: AccountNotifications[] = [ export const mockSingleAccountNotifications: AccountNotifications[] = [ { account: mockGitHubCloudAccount, - notifications: [mockGitHubNotifications[0]], + notifications: [mockSingleNotification], error: null, }, ]; diff --git a/src/renderer/hooks/useNotifications.test.ts b/src/renderer/hooks/useNotifications.test.ts index c1c93e0fb..6498c5559 100644 --- a/src/renderer/hooks/useNotifications.test.ts +++ b/src/renderer/hooks/useNotifications.test.ts @@ -3,8 +3,6 @@ import { act, renderHook, waitFor } from '@testing-library/react'; import axios, { AxiosError } from 'axios'; import nock from 'nock'; -import * as logger from '../../shared/logger'; - import { mockAuth, mockGitHubCloudAccount, @@ -16,16 +14,19 @@ import { mockSingleNotification, } from '../utils/api/__mocks__/response-mocks'; import { Errors } from '../utils/errors'; +import * as logger from '../utils/logger'; import { useNotifications } from './useNotifications'; describe('renderer/hooks/useNotifications.ts', () => { - const logErrorSpy = jest.spyOn(logger, 'logError').mockImplementation(); + const rendererLogErrorSpy = jest + .spyOn(logger, 'rendererLogError') + .mockImplementation(); beforeEach(() => { // axios will default to using the XHR adapter which can't be intercepted // by nock. So, configure axios to use the node adapter. axios.defaults.adapter = 'http'; - logErrorSpy.mockReset(); + rendererLogErrorSpy.mockReset(); }); const id = mockSingleNotification.id; @@ -300,7 +301,7 @@ describe('renderer/hooks/useNotifications.ts', () => { }); expect(result.current.globalError).toBe(Errors.BAD_CREDENTIALS); - expect(logErrorSpy).toHaveBeenCalledTimes(4); + expect(rendererLogErrorSpy).toHaveBeenCalledTimes(4); }); it('should fetch notifications with different failures', async () => { @@ -343,7 +344,7 @@ describe('renderer/hooks/useNotifications.ts', () => { }); expect(result.current.globalError).toBeNull(); - expect(logErrorSpy).toHaveBeenCalledTimes(4); + expect(rendererLogErrorSpy).toHaveBeenCalledTimes(4); }); }); @@ -386,7 +387,7 @@ describe('renderer/hooks/useNotifications.ts', () => { }); expect(result.current.notifications.length).toBe(0); - expect(logErrorSpy).toHaveBeenCalledTimes(1); + expect(rendererLogErrorSpy).toHaveBeenCalledTimes(1); }); }); @@ -429,7 +430,7 @@ describe('renderer/hooks/useNotifications.ts', () => { }); expect(result.current.notifications.length).toBe(0); - expect(logErrorSpy).toHaveBeenCalledTimes(1); + expect(rendererLogErrorSpy).toHaveBeenCalledTimes(1); }); }); @@ -521,7 +522,7 @@ describe('renderer/hooks/useNotifications.ts', () => { }); expect(result.current.notifications.length).toBe(0); - expect(logErrorSpy).toHaveBeenCalledTimes(1); + expect(rendererLogErrorSpy).toHaveBeenCalledTimes(1); }); }); }); diff --git a/src/renderer/hooks/useNotifications.ts b/src/renderer/hooks/useNotifications.ts index b6dd044b6..e6dc1f2f5 100644 --- a/src/renderer/hooks/useNotifications.ts +++ b/src/renderer/hooks/useNotifications.ts @@ -1,7 +1,5 @@ import { useCallback, useState } from 'react'; -import { logError } from '../../shared/logger'; - import type { Account, AccountNotifications, @@ -17,6 +15,7 @@ import { } from '../utils/api/client'; import { updateTrayIcon } from '../utils/comms'; import { isMarkAsDoneFeatureSupported } from '../utils/features'; +import { rendererLogError } from '../utils/logger'; import { triggerNativeNotifications } from '../utils/notifications/native'; import { getAllNotifications, @@ -129,7 +128,7 @@ export const useNotifications = (): NotificationsState => { setNotifications(updatedNotifications); setTrayIconColor(updatedNotifications); } catch (err) { - logError( + rendererLogError( 'markNotificationsAsRead', 'Error occurred while marking notifications as read', err, @@ -167,7 +166,7 @@ export const useNotifications = (): NotificationsState => { setNotifications(updatedNotifications); setTrayIconColor(updatedNotifications); } catch (err) { - logError( + rendererLogError( 'markNotificationsAsDone', 'Error occurred while marking notifications as done', err, @@ -196,7 +195,7 @@ export const useNotifications = (): NotificationsState => { await markNotificationsAsRead(state, [notification]); } } catch (err) { - logError( + rendererLogError( 'unsubscribeNotification', 'Error occurred while unsubscribing from notification thread', err, diff --git a/src/renderer/routes/Accounts.tsx b/src/renderer/routes/Accounts.tsx index 6c7892a64..d1352cc81 100644 --- a/src/renderer/routes/Accounts.tsx +++ b/src/renderer/routes/Accounts.tsx @@ -22,8 +22,6 @@ import { Text, } from '@primer/react'; -import { logError } from '../../shared/logger'; - import { AvatarWithFallback } from '../components/avatars/AvatarWithFallback'; import { Contents } from '../components/layout/Contents'; import { Page } from '../components/layout/Page'; @@ -44,6 +42,7 @@ import { openDeveloperSettings, openHost, } from '../utils/links'; +import { rendererLogError } from '../utils/logger'; import { saveState } from '../utils/storage'; export const AccountsRoute: FC = () => { @@ -98,7 +97,7 @@ export const AccountsRoute: FC = () => { try { await loginWithGitHubApp(); } catch (err) { - logError('loginWithGitHub', 'failed to login with GitHub', err); + rendererLogError('loginWithGitHub', 'failed to login with GitHub', err); } }, []); diff --git a/src/renderer/routes/Login.tsx b/src/renderer/routes/Login.tsx index 675f55fdf..8914f0062 100644 --- a/src/renderer/routes/Login.tsx +++ b/src/renderer/routes/Login.tsx @@ -4,13 +4,12 @@ import { useNavigate } from 'react-router-dom'; import { KeyIcon, MarkGithubIcon, PersonIcon } from '@primer/octicons-react'; import { Button, Heading, Stack, Text } from '@primer/react'; -import { logError } from '../../shared/logger'; - import { LogoIcon } from '../components/icons/LogoIcon'; import { Centered } from '../components/layout/Centered'; import { AppContext } from '../context/App'; import { Size } from '../types'; import { showWindow } from '../utils/comms'; +import { rendererLogError } from '../utils/logger'; export const LoginRoute: FC = () => { const navigate = useNavigate(); @@ -27,7 +26,11 @@ export const LoginRoute: FC = () => { try { await loginWithGitHubApp(); } catch (err) { - logError('loginWithGitHubApp', 'failed to login with GitHub', err); + rendererLogError( + 'loginWithGitHubApp', + 'failed to login with GitHub', + err, + ); } }, [loginWithGitHubApp]); diff --git a/src/renderer/routes/LoginWithOAuthApp.tsx b/src/renderer/routes/LoginWithOAuthApp.tsx index e3a51f8aa..01c28eed4 100644 --- a/src/renderer/routes/LoginWithOAuthApp.tsx +++ b/src/renderer/routes/LoginWithOAuthApp.tsx @@ -18,8 +18,6 @@ import { } from '@primer/react'; import { Banner } from '@primer/react/experimental'; -import { logError } from '../../shared/logger'; - import { Contents } from '../components/layout/Contents'; import { Page } from '../components/layout/Page'; import { Footer } from '../components/primitives/Footer'; @@ -35,6 +33,7 @@ import { } from '../utils/auth/utils'; import { openExternalLink } from '../utils/comms'; import { Constants } from '../utils/constants'; +import { rendererLogError } from '../utils/logger'; interface IFormData { hostname?: Hostname; @@ -120,7 +119,11 @@ export const LoginWithOAuthAppRoute: FC = () => { await loginWithOAuthApp(data as LoginOAuthAppOptions); navigate(-1); } catch (err) { - logError('loginWithOAuthApp', 'Failed to login with OAuth App', err); + rendererLogError( + 'loginWithOAuthApp', + 'Failed to login with OAuth App', + err, + ); setErrors({ invalidCredentialsForHost: `Failed to validate provided Client ID and Secret against ${data.hostname}`, }); diff --git a/src/renderer/routes/LoginWithPersonalAccessToken.tsx b/src/renderer/routes/LoginWithPersonalAccessToken.tsx index b7729f1ff..eb43b74e4 100644 --- a/src/renderer/routes/LoginWithPersonalAccessToken.tsx +++ b/src/renderer/routes/LoginWithPersonalAccessToken.tsx @@ -18,8 +18,6 @@ import { } from '@primer/react'; import { Banner } from '@primer/react/experimental'; -import { logError } from '../../shared/logger'; - import { Contents } from '../components/layout/Contents'; import { Page } from '../components/layout/Page'; import { Footer } from '../components/primitives/Footer'; @@ -35,6 +33,7 @@ import { } from '../utils/auth/utils'; import { openExternalLink } from '../utils/comms'; import { Constants } from '../utils/constants'; +import { rendererLogError } from '../utils/logger'; interface IFormData { token?: Token; @@ -112,7 +111,7 @@ export const LoginWithPersonalAccessTokenRoute: FC = () => { ); navigate(-1); } catch (err) { - logError( + rendererLogError( 'loginWithPersonalAccessToken', 'Failed to login with PAT', err, diff --git a/src/renderer/utils/api/client.test.ts b/src/renderer/utils/api/client.test.ts index 5b85d285d..87dfe326b 100644 --- a/src/renderer/utils/api/client.test.ts +++ b/src/renderer/utils/api/client.test.ts @@ -1,13 +1,12 @@ import axios, { type AxiosPromise, type AxiosResponse } from 'axios'; -import * as logger from '../../../shared/logger'; - import { mockGitHubCloudAccount, mockGitHubEnterpriseServerAccount, mockToken, } from '../../__mocks__/state-mocks'; import type { Hostname, Link, SettingsState, Token } from '../../types'; +import * as logger from '../../utils/logger'; import { getAuthenticatedUser, getHtmlUrl, @@ -322,7 +321,9 @@ describe('renderer/utils/api/client.ts', () => { }); it('should handle error', async () => { - const logErrorSpy = jest.spyOn(logger, 'logError').mockImplementation(); + const rendererLogErrorSpy = jest + .spyOn(logger, 'rendererLogError') + .mockImplementation(); const apiRequestAuthMock = jest.spyOn(apiRequests, 'apiRequestAuth'); @@ -335,7 +336,7 @@ describe('renderer/utils/api/client.ts', () => { '123' as Token, ); - expect(logErrorSpy).toHaveBeenCalledTimes(1); + expect(rendererLogErrorSpy).toHaveBeenCalledTimes(1); }); }); }); diff --git a/src/renderer/utils/api/client.ts b/src/renderer/utils/api/client.ts index 2c3485f2c..fa6eb1b12 100644 --- a/src/renderer/utils/api/client.ts +++ b/src/renderer/utils/api/client.ts @@ -1,8 +1,6 @@ import type { AxiosPromise } from 'axios'; import { print } from 'graphql/language/printer'; -import { logError } from '../../../shared/logger'; - import type { Account, Hostname, @@ -25,6 +23,7 @@ import type { UserDetails, } from '../../typesGitHub'; import { isAnsweredDiscussionFeatureSupported } from '../features'; +import { rendererLogError } from '../logger'; import { QUERY_SEARCH_DISCUSSIONS } from './graphql/discussions'; import { formatAsGitHubSearchSyntax } from './graphql/utils'; import { apiRequestAuth } from './request'; @@ -219,7 +218,7 @@ export async function getHtmlUrl(url: Link, token: Token): Promise { const response = (await apiRequestAuth(url, 'GET', token)).data; return response.html_url; } catch (err) { - logError( + rendererLogError( 'getHtmlUrl', `error occurred while fetching html url for ${url}`, err, @@ -274,7 +273,7 @@ export async function getLatestDiscussion( )[0] ?? null ); } catch (err) { - logError( + rendererLogError( 'getLatestDiscussion', 'failed to fetch latest discussion for notification', err, diff --git a/src/renderer/utils/api/request.ts b/src/renderer/utils/api/request.ts index 324d4844f..a3d445015 100644 --- a/src/renderer/utils/api/request.ts +++ b/src/renderer/utils/api/request.ts @@ -4,10 +4,9 @@ import axios, { type Method, } from 'axios'; -import { logError } from '../../../shared/logger'; - import type { Link, Token } from '../../types'; import { decryptValue } from '../comms'; +import { rendererLogError } from '../logger'; import { getNextURLFromLinkHeader } from './utils'; /** @@ -70,7 +69,7 @@ export async function apiRequestAuth( nextUrl = getNextURLFromLinkHeader(response); } } catch (err) { - logError('apiRequestAuth', 'API request failed:', err); + rendererLogError('apiRequestAuth', 'API request failed:', err); throw err; } diff --git a/src/renderer/utils/auth/utils.ts b/src/renderer/utils/auth/utils.ts index c1b84add4..3c01a78f1 100644 --- a/src/renderer/utils/auth/utils.ts +++ b/src/renderer/utils/auth/utils.ts @@ -5,7 +5,6 @@ import semver from 'semver'; import { APPLICATION } from '../../../shared/constants'; import { namespacedEvent } from '../../../shared/events'; -import { logError, logInfo, logWarn } from '../../../shared/logger'; import type { Account, @@ -23,6 +22,7 @@ import { apiRequest } from '../api/request'; import { encryptValue, openExternalLink } from '../comms'; import { Constants } from '../constants'; import { getPlatformFromHostname } from '../helpers'; +import { rendererLogError, rendererLogInfo, rendererLogWarn } from '../logger'; import type { AuthMethod, AuthResponse, AuthTokenResponse } from './types'; export function authGitHub( @@ -69,7 +69,7 @@ export function authGitHub( ipcRenderer.on( namespacedEvent('auth-callback'), (_, callbackUrl: string) => { - logInfo( + rendererLogInfo( 'renderer:auth-callback', `received authentication callback URL ${callbackUrl}`, ); @@ -137,7 +137,7 @@ export async function addAccount( ); if (accountAlreadyExists) { - logWarn( + rendererLogWarn( 'addAccount', `account for user ${newAccount.user.login} already exists`, ); @@ -189,13 +189,13 @@ export async function refreshAccount(account: Account): Promise { ); if (!account.hasRequiredScopes) { - logWarn( + rendererLogWarn( 'refreshAccount', `account for user ${account.user.login} is missing required scopes`, ); } } catch (err) { - logError( + rendererLogError( 'refreshAccount', `failed to refresh account for user ${account.user.login}`, err, diff --git a/src/renderer/utils/helpers.test.ts b/src/renderer/utils/helpers.test.ts index fe1975d95..c1b64eee8 100644 --- a/src/renderer/utils/helpers.test.ts +++ b/src/renderer/utils/helpers.test.ts @@ -6,11 +6,10 @@ import { import type { AxiosPromise, AxiosResponse } from 'axios'; -import * as logger from '../../shared/logger'; - import { mockPersonalAccessTokenAccount } from '../__mocks__/state-mocks'; import type { Hostname, Link } from '../types'; import type { SubjectType } from '../typesGitHub'; +import * as logger from '../utils/logger'; import { mockGraphQLResponse, mockSingleNotification, @@ -484,7 +483,9 @@ describe('renderer/utils/helpers.ts', () => { }); it('defaults when exception handled during specialized html enrichment process', async () => { - const logErrorSpy = jest.spyOn(logger, 'logError').mockImplementation(); + const rendererLogErrorSpy = jest + .spyOn(logger, 'rendererLogError') + .mockImplementation(); const subject = { title: 'generate github web url unit tests', @@ -512,7 +513,7 @@ describe('renderer/utils/helpers.ts', () => { expect(result).toBe( `https://github.com/gitify-app/notifications-test?${mockNotificationReferrer}`, ); - expect(logErrorSpy).toHaveBeenCalledTimes(2); + expect(rendererLogErrorSpy).toHaveBeenCalledTimes(2); }); }); }); diff --git a/src/renderer/utils/helpers.ts b/src/renderer/utils/helpers.ts index 838ffe023..772cdccb8 100644 --- a/src/renderer/utils/helpers.ts +++ b/src/renderer/utils/helpers.ts @@ -4,13 +4,12 @@ import { ChevronRightIcon, } from '@primer/octicons-react'; -import { logError, logWarn } from '../../shared/logger'; - import type { Chevron, Hostname, Link } from '../types'; import type { Notification } from '../typesGitHub'; import { getHtmlUrl, getLatestDiscussion } from './api/client'; import type { PlatformType } from './auth/types'; import { Constants } from './constants'; +import { rendererLogError, rendererLogWarn } from './logger'; import { getCheckSuiteAttributes } from './notifications/handlers/checkSuite'; import { getClosestDiscussionCommentOrReply } from './notifications/handlers/discussion'; import { getWorkflowRunAttributes } from './notifications/handlers/workflowRun'; @@ -151,14 +150,14 @@ export async function generateGitHubWebUrl( } } } catch (err) { - logError( + rendererLogError( 'generateGitHubWebUrl', 'Failed to resolve specific notification html url for', err, notification, ); - logWarn( + rendererLogWarn( 'generateGitHubWebUrl', `Falling back to repository root url: ${notification.repository.full_name}`, ); diff --git a/src/renderer/utils/logger.test.ts b/src/renderer/utils/logger.test.ts new file mode 100644 index 000000000..2ae2b88a6 --- /dev/null +++ b/src/renderer/utils/logger.test.ts @@ -0,0 +1,50 @@ +import log from 'electron-log'; + +import { mockSingleNotification } from './api/__mocks__/response-mocks'; +import { rendererLogError, rendererLogInfo, rendererLogWarn } from './logger'; + +describe('renderer/utils/logger.ts', () => { + const logInfoSpy = jest.spyOn(log, 'info').mockImplementation(); + const logWarnSpy = jest.spyOn(log, 'warn').mockImplementation(); + const logErrorSpy = jest.spyOn(log, 'error').mockImplementation(); + const mockError = new Error('boom'); + + beforeEach(() => { + logInfoSpy.mockReset(); + logWarnSpy.mockReset(); + logErrorSpy.mockReset(); + }); + + it('logs info without notification', () => { + rendererLogInfo('foo', 'bar'); + expect(logInfoSpy).toHaveBeenCalledWith('[foo]', 'bar'); + }); + + it('logs info with notification', () => { + rendererLogInfo('foo', 'bar', mockSingleNotification); + expect(logInfoSpy).toHaveBeenCalledWith( + '[foo]', + 'bar', + '[Issue >> gitify-app/notifications-test >> I am a robot and this is a test!]', + ); + }); + + it('logs warn with notification', () => { + rendererLogWarn('foo', 'bar', mockSingleNotification); + expect(logWarnSpy).toHaveBeenCalledWith( + '[foo]', + 'bar', + '[Issue >> gitify-app/notifications-test >> I am a robot and this is a test!]', + ); + }); + + it('logs error with notification', () => { + rendererLogError('foo', 'bar', mockError, mockSingleNotification); + expect(logErrorSpy).toHaveBeenCalledWith( + '[foo]', + 'bar', + '[Issue >> gitify-app/notifications-test >> I am a robot and this is a test!]', + mockError, + ); + }); +}); diff --git a/src/renderer/utils/logger.ts b/src/renderer/utils/logger.ts new file mode 100644 index 000000000..02aab5bf2 --- /dev/null +++ b/src/renderer/utils/logger.ts @@ -0,0 +1,41 @@ +import { logError, logInfo, logWarn } from '../../shared/logger'; + +import type { Notification } from '../typesGitHub'; + +// Renderer logger augments log entries with notification context formatting. +export function rendererLogInfo( + type: string, + message: string, + notification?: Notification, +) { + logInfo(type, message, buildContexts(notification)); +} + +export function rendererLogWarn( + type: string, + message: string, + notification?: Notification, +) { + logWarn(type, message, buildContexts(notification)); +} + +export function rendererLogError( + type: string, + message: string, + err: Error, + notification?: Notification, +) { + logError(type, message, err, buildContexts(notification)); +} + +function buildContexts(notification?: Notification): string[] { + if (!notification) { + return []; + } + + return [ + notification.subject.type, + notification.repository.full_name, + notification.subject.title, + ]; +} diff --git a/src/renderer/utils/notifications/native.test.ts b/src/renderer/utils/notifications/native.test.ts index e075452e1..65ec46939 100644 --- a/src/renderer/utils/notifications/native.test.ts +++ b/src/renderer/utils/notifications/native.test.ts @@ -5,7 +5,10 @@ import { import { mockAuth } from '../../__mocks__/state-mocks'; import { defaultSettings } from '../../context/defaults'; import type { SettingsState } from '../../types'; -import { mockGitHubNotifications } from '../api/__mocks__/response-mocks'; +import { + mockGitHubNotifications, + mockSingleNotification, +} from '../api/__mocks__/response-mocks'; import * as comms from '../comms'; import * as links from '../links'; import * as native from './native'; @@ -115,13 +118,13 @@ describe('renderer/utils/notifications/native.ts', () => { jest.spyOn(links, 'openNotification'); const nativeNotification: Notification = native.raiseNativeNotification([ - mockGitHubNotifications[0], + mockSingleNotification, ]); nativeNotification.onclick(null); expect(links.openNotification).toHaveBeenCalledTimes(1); expect(links.openNotification).toHaveBeenLastCalledWith( - mockGitHubNotifications[0], + mockSingleNotification, ); expect(hideWindowMock).toHaveBeenCalledTimes(1); }); diff --git a/src/renderer/utils/notifications/notifications.test.ts b/src/renderer/utils/notifications/notifications.test.ts index 498bae3c0..2bdd91978 100644 --- a/src/renderer/utils/notifications/notifications.test.ts +++ b/src/renderer/utils/notifications/notifications.test.ts @@ -1,13 +1,12 @@ import axios from 'axios'; import nock from 'nock'; -import * as logger from '../../../shared/logger'; - import { mockSingleAccountNotifications } from '../../__mocks__/notifications-mocks'; import { partialMockNotification } from '../../__mocks__/partial-mocks'; import { mockSettings } from '../../__mocks__/state-mocks'; import type { Link } from '../../types'; import type { Repository } from '../../typesGitHub'; +import * as logger from '../../utils/logger'; import { enrichNotification, getNotificationCount } from './notifications'; describe('renderer/utils/notifications/notifications.ts', () => { @@ -28,7 +27,9 @@ describe('renderer/utils/notifications/notifications.ts', () => { }); it('enrichNotification - catches error and logs message', async () => { - const logErrorSpy = jest.spyOn(logger, 'logError').mockImplementation(); + const rendererLogErrorSpy = jest + .spyOn(logger, 'rendererLogError') + .mockImplementation(); const mockError = new Error('Test error'); const mockNotification = partialMockNotification({ @@ -47,7 +48,7 @@ describe('renderer/utils/notifications/notifications.ts', () => { await enrichNotification(mockNotification, mockSettings); - expect(logErrorSpy).toHaveBeenCalledWith( + expect(rendererLogErrorSpy).toHaveBeenCalledWith( 'enrichNotification', 'failed to enrich notification details for', mockError, diff --git a/src/renderer/utils/notifications/notifications.ts b/src/renderer/utils/notifications/notifications.ts index a1de6ba86..69f630c52 100644 --- a/src/renderer/utils/notifications/notifications.ts +++ b/src/renderer/utils/notifications/notifications.ts @@ -1,5 +1,3 @@ -import { logError, logWarn } from '../../../shared/logger'; - import type { AccountNotifications, GitifyState, @@ -9,6 +7,7 @@ import type { GitifySubject, Notification } from '../../typesGitHub'; import { listNotificationsForAuthenticatedUser } from '../api/client'; import { determineFailureType } from '../api/errors'; import { updateTrayIcon } from '../comms'; +import { rendererLogError, rendererLogWarn } from '../logger'; import { filterBaseNotifications, filterDetailedNotifications, @@ -78,7 +77,7 @@ export async function getAllNotifications( error: null, }; } catch (err) { - logError( + rendererLogError( 'getAllNotifications', 'error occurred while fetching account notifications', err, @@ -123,14 +122,17 @@ export async function enrichNotification( const handler = createNotificationHandler(notification); additionalSubjectDetails = await handler.enrich(notification, settings); } catch (err) { - logError( + rendererLogError( 'enrichNotification', 'failed to enrich notification details for', err, notification, ); - logWarn('enrichNotification', 'Continuing with base notification details'); + rendererLogWarn( + 'enrichNotification', + 'Continuing with base notification details', + ); } return { diff --git a/src/shared/logger.test.ts b/src/shared/logger.test.ts index 468741f2f..40447650f 100644 --- a/src/shared/logger.test.ts +++ b/src/shared/logger.test.ts @@ -1,9 +1,8 @@ import log from 'electron-log'; -import { mockSingleNotification } from '../renderer/utils/api/__mocks__/response-mocks'; import { logError, logInfo, logWarn } from './logger'; -describe('renderer/utils/logger.ts', () => { +describe('shared/logger.ts', () => { const logInfoSpy = jest.spyOn(log, 'info').mockImplementation(); const logWarnSpy = jest.spyOn(log, 'warn').mockImplementation(); const logErrorSpy = jest.spyOn(log, 'error').mockImplementation(); @@ -17,61 +16,70 @@ describe('renderer/utils/logger.ts', () => { }); describe('logInfo', () => { - it('log info without notification', () => { + it('logs info without contexts', () => { logInfo('foo', 'bar'); - expect(logInfoSpy).toHaveBeenCalledTimes(1); expect(logInfoSpy).toHaveBeenCalledWith('[foo]', 'bar'); }); - it('log info with notification', () => { - logInfo('foo', 'bar', mockSingleNotification); + it('logs info with single context', () => { + logInfo('foo', 'bar', ['ctx']); + expect(logInfoSpy).toHaveBeenCalledTimes(1); + expect(logInfoSpy).toHaveBeenCalledWith('[foo]', 'bar', '[ctx]'); + }); + it('logs info with multiple contexts', () => { + logInfo('foo', 'bar', ['ctx1', 'ctx2']); expect(logInfoSpy).toHaveBeenCalledTimes(1); - expect(logInfoSpy).toHaveBeenCalledWith( - '[foo]', - 'bar', - '[Issue >> gitify-app/notifications-test >> I am a robot and this is a test!]', - ); + expect(logInfoSpy).toHaveBeenCalledWith('[foo]', 'bar', '[ctx1 >> ctx2]'); }); }); describe('logWarn', () => { - it('log warn without notification', () => { + it('logs warn without contexts', () => { logWarn('foo', 'bar'); - expect(logWarnSpy).toHaveBeenCalledTimes(1); expect(logWarnSpy).toHaveBeenCalledWith('[foo]', 'bar'); }); - it('log warn with notification', () => { - logWarn('foo', 'bar', mockSingleNotification); + it('logs warn with single context', () => { + logWarn('foo', 'bar', ['ctx']); + expect(logWarnSpy).toHaveBeenCalledTimes(1); + expect(logWarnSpy).toHaveBeenCalledWith('[foo]', 'bar', '[ctx]'); + }); + it('logs warn with multiple contexts', () => { + logWarn('foo', 'bar', ['ctx1', 'ctx2']); expect(logWarnSpy).toHaveBeenCalledTimes(1); - expect(logWarnSpy).toHaveBeenCalledWith( - '[foo]', - 'bar', - '[Issue >> gitify-app/notifications-test >> I am a robot and this is a test!]', - ); + expect(logWarnSpy).toHaveBeenCalledWith('[foo]', 'bar', '[ctx1 >> ctx2]'); }); }); describe('logError', () => { - it('log error without notification', () => { + it('logs error without contexts', () => { logError('foo', 'bar', mockError); - expect(logErrorSpy).toHaveBeenCalledTimes(1); expect(logErrorSpy).toHaveBeenCalledWith('[foo]', 'bar', mockError); }); - it('log error with notification', () => { - logError('foo', 'bar', mockError, mockSingleNotification); + it('logs error with single context', () => { + logError('foo', 'bar', mockError, ['ctx']); + expect(logErrorSpy).toHaveBeenCalledTimes(1); + expect(logErrorSpy).toHaveBeenCalledWith( + '[foo]', + 'bar', + '[ctx]', + mockError, + ); + }); + it('logs error with multiple contexts', () => { + logError('foo', 'bar', mockError, ['ctx1', 'ctx2']); expect(logErrorSpy).toHaveBeenCalledTimes(1); expect(logErrorSpy).toHaveBeenCalledWith( '[foo]', 'bar', - '[Issue >> gitify-app/notifications-test >> I am a robot and this is a test!]', + '[ctx1 >> ctx2]', mockError, ); }); diff --git a/src/shared/logger.ts b/src/shared/logger.ts index bd1e978d2..c52388e31 100644 --- a/src/shared/logger.ts +++ b/src/shared/logger.ts @@ -1,46 +1,44 @@ import log from 'electron-log'; -import type { Notification } from '../renderer/typesGitHub'; +type AllowedLogFunction = typeof log.info | typeof log.warn | typeof log.error; export function logInfo( type: string, message: string, - notification?: Notification, + contexts: string[] = [], ) { - logMessage(log.info, type, message, null, notification); + logMessage(log.info, type, message, undefined, contexts); } export function logWarn( type: string, message: string, - notification?: Notification, + contexts: string[] = [], ) { - logMessage(log.warn, type, message, null, notification); + logMessage(log.warn, type, message, undefined, contexts); } export function logError( type: string, message: string, err: Error, - notification?: Notification, + contexts: string[] = [], ) { - logMessage(log.error, type, message, err, notification); + logMessage(log.error, type, message, err, contexts); } function logMessage( - // biome-ignore lint/suspicious/noExplicitAny: Allow any for logging purposes - logFunction: (...params: any[]) => void, + logFunction: AllowedLogFunction, type: string, message: string, err?: Error, - notification?: Notification, + contexts: string[] = [], ) { const args: (string | Error)[] = [`[${type}]`, message]; - if (notification) { - args.push( - `[${notification.subject.type} >> ${notification.repository.full_name} >> ${notification.subject.title}]`, - ); + if (contexts.length) { + const combined = contexts.join(' >> '); + args.push(`[${combined}]`); } if (err) { diff --git a/src/shared/platform.test.ts b/src/shared/platform.test.ts new file mode 100644 index 000000000..708a0492f --- /dev/null +++ b/src/shared/platform.test.ts @@ -0,0 +1,36 @@ +import { isLinux, isMacOS, isWindows } from './platform'; + +describe('shared/platform.ts', () => { + const originalPlatform = process.platform; + + function mockPlatform(value: NodeJS.Platform) { + Object.defineProperty(process, 'platform', { + value, + }); + } + + afterAll(() => { + mockPlatform(originalPlatform as NodeJS.Platform); + }); + + it('isLinux returns true only for linux', () => { + mockPlatform('linux'); + expect(isLinux()).toBe(true); + mockPlatform('darwin'); + expect(isLinux()).toBe(false); + }); + + it('isMacOS returns true only for darwin', () => { + mockPlatform('darwin'); + expect(isMacOS()).toBe(true); + mockPlatform('win32'); + expect(isMacOS()).toBe(false); + }); + + it('isWindows returns true only for win32', () => { + mockPlatform('win32'); + expect(isWindows()).toBe(true); + mockPlatform('linux'); + expect(isWindows()).toBe(false); + }); +});