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

Improve cleanup of tests #543

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions src/hooks/useAsyncValue.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,8 @@

import type { DependencyList } from 'react';

import { useInstantEffect } from './useInstantEffect.js';
import { useAsyncCallback, type AsyncCallbackState } from './useAsyncCallback.js';
import { useInstantEffect } from './useInstantEffect.js';

/**
* A hook that allows to execute an asynchronous function and provides the state of the execution.
Expand Down Expand Up @@ -37,8 +37,8 @@ import { useAsyncCallback, type AsyncCallbackState } from './useAsyncCallback.js
* }
* ```
*/
export const useAsyncValue = <A extends Array<unknown>, R>(
callback: ( ...args: A ) => Promise<R>,
export const useAsyncValue = <R>(
callback: () => Promise<R>,
deps: DependencyList
): AsyncValueHookResult<R> => {
const [ asyncCallback, asyncState ] = useAsyncCallback( callback );
Expand Down
1 change: 1 addition & 0 deletions src/useMultiRootEditor.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -396,6 +396,7 @@ const useMultiRootEditor = ( props: MultiRootHookProps ): MultiRootHookReturns =
} );

setTimeout( () => {
/* istanbul ignore next -- @preserve */
if ( props.onReady ) {
props.onReady( watchdog!.editor );
}
Expand Down
22 changes: 22 additions & 0 deletions tests/_utils/turnOffErrors.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
/**
* @license Copyright (c) 2003-2024, CKSource Holding sp. z o.o. All rights reserved.
* For licensing, see LICENSE.md.
*/

import type { Awaitable } from '@ckeditor/ckeditor5-integrations-common';
import { timeout } from './timeout.js';

export async function turnOffErrors( callback: () => Awaitable<void> ): Promise<void> {
const handler = ( evt: ErrorEvent ) => {
evt.preventDefault();
};

window.addEventListener( 'error', handler, { capture: true, once: true } );
Copy link
Member

Choose a reason for hiding this comment

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

To my understanding, this mutes Vitest errors. Would it be possible to use expect().toThrowError() instead?

Copy link
Member Author

Choose a reason for hiding this comment

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

It's not working, I tested it


try {
await callback();
await timeout( 150 );
} finally {
window.removeEventListener( 'error', handler );
}
}
19 changes: 0 additions & 19 deletions tests/_utils/turnoffdefaulterrorcatching.js

This file was deleted.

9 changes: 5 additions & 4 deletions tests/ckeditor.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -13,12 +13,12 @@ import MockedEditor from './_utils/editor.js';
import { timeout } from './_utils/timeout.js';
import { createDefer } from './_utils/defer.js';
import { PromiseManager } from './_utils/promisemanager.js';
import turnOffDefaultErrorCatching from './_utils/turnoffdefaulterrorcatching.js';
import CKEditor, { type Props } from '../src/ckeditor.js';
import { expectToBeTruthy } from './_utils/expectToBeTruthy.js';

import type { LifeCycleElementSemaphore } from '../src/lifecycle/LifeCycleElementSemaphore.js';
import type { EditorSemaphoreMountResult } from '../src/lifecycle/LifeCycleEditorSemaphore.js';
import { turnOffErrors } from './_utils/turnOffErrors.js';

const MockEditor = MockedEditor as any;

Expand Down Expand Up @@ -1055,13 +1055,14 @@ describe( '<CKEditor> Component', () => {

expect( firstEditor ).to.be.instanceOf( MockEditor );

await turnOffDefaultErrorCatching( () => {
return new Promise( res => {
await manager.all();
await turnOffErrors( async () => {
await new Promise( resolve => {
component?.rerender(
<CKEditor
ref={instanceRef}
editor={MockEditor}
onReady={res}
onReady={resolve}
onAfterDestroy={onAfterDestroySpy}
/>
);
Expand Down
21 changes: 10 additions & 11 deletions tests/cloud/useCKEditorCloud.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -4,16 +4,15 @@
*/

import { afterEach, describe, expect, expectTypeOf, it } from 'vitest';
import { renderHook, waitFor, act, cleanup } from '@testing-library/react';
import { renderHook, waitFor, act } from '@testing-library/react';

import type { CKEditorCloudConfig } from '@ckeditor/ckeditor5-integrations-common';
import { removeAllCkCdnResources } from '@ckeditor/ckeditor5-integrations-common/test-utils';

import useCKEditorCloud from '../../src/cloud/useCKEditorCloud.js';

describe( 'useCKEditorCloud', () => {
describe( 'useCKEditorCloud', { timeout: 8000 }, () => {
afterEach( () => {
cleanup();
removeAllCkCdnResources();
} );

Expand All @@ -31,7 +30,7 @@ describe( 'useCKEditorCloud', () => {
if ( result.current.status === 'success' ) {
expect( result.current.CKEditor ).toBeDefined();
}
} );
}, { timeout: 5000 } );
} );

it( 'should load additional bundle after updating deps', async () => {
Expand All @@ -52,7 +51,7 @@ describe( 'useCKEditorCloud', () => {
expect( result.current.CKEditor ).toBeDefined();
expect( result.current.CKEditorPremiumFeatures ).toBeUndefined();
}
} );
}, { timeout: 5000 } );

rerender( {
version: '43.0.0',
Expand All @@ -70,7 +69,7 @@ describe( 'useCKEditorCloud', () => {
expect( result.current.CKEditor ).toBeDefined();
expect( result.current.CKEditorPremiumFeatures ).toBeDefined();
}
} );
}, { timeout: 5000 } );
} );

describe( 'typings', () => {
Expand All @@ -82,7 +81,7 @@ describe( 'useCKEditorCloud', () => {

await waitFor( () => {
expect( result.current.status ).toBe( 'success' );
} );
}, { timeout: 5000 } );

if ( result.current.status === 'success' ) {
expectTypeOf( result.current.CKEditorPremiumFeatures ).not.toBeNullable();
Expand All @@ -97,7 +96,7 @@ describe( 'useCKEditorCloud', () => {

await waitFor( () => {
expect( result.current.status ).toBe( 'success' );
} );
}, { timeout: 5000 } );

if ( result.current.status === 'success' ) {
expectTypeOf( result.current.CKEditorPremiumFeatures ).toBeNullable();
Expand All @@ -111,7 +110,7 @@ describe( 'useCKEditorCloud', () => {

await waitFor( () => {
expect( result.current.status ).toBe( 'success' );
} );
}, { timeout: 5000 } );

if ( result.current.status === 'success' ) {
expectTypeOf( result.current.CKEditorPremiumFeatures ).toBeNullable();
Expand All @@ -128,7 +127,7 @@ describe( 'useCKEditorCloud', () => {

await waitFor( () => {
expect( result.current.status ).toBe( 'success' );
} );
}, { timeout: 5000 } );

if ( result.current.status === 'success' ) {
expectTypeOf( result.current.CKBox ).not.toBeNullable();
Expand All @@ -142,7 +141,7 @@ describe( 'useCKEditorCloud', () => {

await waitFor( () => {
expect( result.current.status ).toBe( 'success' );
} );
}, { timeout: 5000 } );

if ( result.current.status === 'success' ) {
expectTypeOf( result.current.CKBox ).toBeNullable();
Expand Down
5 changes: 2 additions & 3 deletions tests/cloud/withCKEditorCloud.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -5,20 +5,19 @@

import React, { type MutableRefObject } from 'react';
import { afterEach, describe, expect, it } from 'vitest';
import { cleanup, render } from '@testing-library/react';
import { render } from '@testing-library/react';

import { createDefer } from '@ckeditor/ckeditor5-integrations-common';
import { removeAllCkCdnResources } from '@ckeditor/ckeditor5-integrations-common/test-utils';

import withCKEditorCloud, { type WithCKEditorCloudHocProps } from '../../src/cloud/withCKEditorCloud.js';

describe( 'withCKEditorCloud', () => {
describe( 'withCKEditorCloud', { timeout: 5000 }, () => {
const lastRenderedMockProps: MutableRefObject<WithCKEditorCloudHocProps | null> = {
current: null
};

afterEach( () => {
cleanup();
removeAllCkCdnResources();
lastRenderedMockProps.current = null;
} );
Expand Down
11 changes: 5 additions & 6 deletions tests/context/ckeditorcontext.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,10 @@ import CKEditorContext, {
import CKEditor from '../../src/ckeditor.js';
import MockedEditor from '../_utils/editor.js';
import { ClassicEditor, ContextWatchdog, CKEditorError } from 'ckeditor5';
import turnOffDefaultErrorCatching from '../_utils/turnoffdefaulterrorcatching.js';
import ContextMock, { DeferredContextMock } from '../_utils/context.js';
import { timeout } from '../_utils/timeout.js';
import { PromiseManager } from '../_utils/promisemanager.js';
import { turnOffErrors } from '../_utils/turnOffErrors.js';

const MockEditor = MockedEditor as any;

Expand Down Expand Up @@ -303,20 +303,19 @@ describe( '<CKEditorContext> Component', () => {
} );

const { watchdog } = contextRef.current as ExtractContextWatchdogValueByStatus<'initialized'>;
const error = new CKEditorError( 'foo', watchdog.context );

await turnOffDefaultErrorCatching( async () => {
await turnOffErrors( async () => {
const error = new CKEditorError( 'foo', watchdog.context );

setTimeout( () => {
throw error;
} );

await timeout( 150 );
} );

expect( onErrorSpy ).toHaveBeenCalledOnce();
const errorEventArgs = onErrorSpy.mock.calls[ 0 ];

expect( errorEventArgs[ 0 ] ).to.equal( error );
expect( errorEventArgs[ 0 ] ).to.instanceOf( Error );
expect( errorEventArgs[ 1 ] ).to.deep.equal( {
phase: 'runtime',
willContextRestart: true
Expand Down
6 changes: 2 additions & 4 deletions tests/hooks/useAsyncCallback.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,14 +3,12 @@
* For licensing, see LICENSE.md.
*/

import { afterEach, describe, expect, it, vi } from 'vitest';
import { renderHook, act, waitFor, cleanup } from '@testing-library/react';
import { describe, expect, it, vi } from 'vitest';
import { renderHook, act, waitFor } from '@testing-library/react';
import { useAsyncCallback } from '../../src/hooks/useAsyncCallback.js';
import { timeout } from '../_utils/timeout.js';

describe( 'useAsyncCallback', () => {
afterEach( cleanup );

it( 'should execute the callback and update the state correctly when the callback resolves', async () => {
const fetchData = vi.fn().mockResolvedValue( 'data' );

Expand Down
6 changes: 2 additions & 4 deletions tests/hooks/useAsyncValue.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,11 @@
* For licensing, see LICENSE.md.
*/

import { afterEach, describe, expect, it } from 'vitest';
import { cleanup, renderHook, waitFor } from '@testing-library/react';
import { describe, expect, it } from 'vitest';
import { renderHook, waitFor } from '@testing-library/react';
import { useAsyncValue } from '../../src/hooks/useAsyncValue.js';

describe( 'useAsyncValue', () => {
afterEach( cleanup );

it( 'should return a mutable ref object', async () => {
const { result } = renderHook( () => useAsyncValue( async () => 123, [] ) );

Expand Down
6 changes: 2 additions & 4 deletions tests/hooks/useInstantEditorEffect.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,11 @@
* For licensing, see LICENSE.md.
*/

import { afterEach, describe, expect, it, vi } from 'vitest';
import { cleanup, renderHook } from '@testing-library/react';
import { describe, expect, it, vi } from 'vitest';
import { renderHook } from '@testing-library/react';
import { useInstantEditorEffect } from '../../src/hooks/useInstantEditorEffect.js';

describe( 'useInstantEditorEffect', () => {
afterEach( cleanup );

it( 'should execute the provided function after mounting of editor', () => {
const semaphore = {
runAfterMount: vi.fn()
Expand Down
6 changes: 2 additions & 4 deletions tests/hooks/useInstantEffect.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,11 @@
* For licensing, see LICENSE.md.
*/

import { afterEach, describe, expect, it, vi } from 'vitest';
import { cleanup, renderHook } from '@testing-library/react';
import { describe, expect, it, vi } from 'vitest';
import { renderHook } from '@testing-library/react';
import { useInstantEffect } from '../../src/hooks/useInstantEffect.js';

describe( 'useInstantEffect', () => {
afterEach( cleanup );

it( 'should call the effect function when dependencies change', () => {
const effectFn = vi.fn();
const { rerender } = renderHook( deps => useInstantEffect( effectFn, deps ), {
Expand Down
6 changes: 2 additions & 4 deletions tests/hooks/useIsMountedRef.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,11 @@
* For licensing, see LICENSE.md.
*/

import { afterEach, describe, expect, it } from 'vitest';
import { cleanup, renderHook } from '@testing-library/react';
import { describe, expect, it } from 'vitest';
import { renderHook } from '@testing-library/react';
import { useIsMountedRef } from '../../src/hooks/useIsMountedRef.js';

describe( 'useIsMountedRef', () => {
afterEach( cleanup );

it( 'should return a mutable ref object', () => {
const { result } = renderHook( () => useIsMountedRef() );

Expand Down
6 changes: 2 additions & 4 deletions tests/hooks/useIsUnmountedRef.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,11 @@
* For licensing, see LICENSE.md.
*/

import { afterEach, describe, expect, it } from 'vitest';
import { cleanup, renderHook } from '@testing-library/react';
import { describe, expect, it } from 'vitest';
import { renderHook } from '@testing-library/react';
import { useIsUnmountedRef } from '../../src/hooks/useIsUnmountedRef.js';

describe( 'useIsUnmountedRef', () => {
afterEach( cleanup );

it( 'should return a mutable ref object', () => {
const { result } = renderHook( () => useIsUnmountedRef() );

Expand Down
6 changes: 2 additions & 4 deletions tests/hooks/useRefSafeCallback.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,11 @@
* For licensing, see LICENSE.md.
*/

import { expect, it, describe, vi, afterEach } from 'vitest';
import { renderHook, act, cleanup } from '@testing-library/react';
import { expect, it, describe, vi } from 'vitest';
import { renderHook, act } from '@testing-library/react';
import { useRefSafeCallback } from '../../src/hooks/useRefSafeCallback.js';

describe( 'useRefSafeCallback', () => {
afterEach( cleanup );

it( 'should return a function', () => {
const { result } = renderHook( () => useRefSafeCallback( () => {} ) );

Expand Down
Loading