codeant-ai-for-open-source[bot] commented on code in PR #37367: URL: https://github.com/apache/superset/pull/37367#discussion_r2755475455
########## superset-frontend/src/embedded/initEmbedded.test.ts: ########## @@ -0,0 +1,95 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +import { initFeatureFlags } from '@superset-ui/core'; +import getBootstrapData from 'src/utils/getBootstrapData'; + +jest.mock('@superset-ui/core', () => ({ + ...jest.requireActual('@superset-ui/core'), + initFeatureFlags: jest.fn(), +})); + +jest.mock('src/utils/getBootstrapData', () => ({ + __esModule: true, + default: jest.fn(), +})); + +const mockInitFeatureFlags = initFeatureFlags as jest.MockedFunction< + typeof initFeatureFlags +>; +const mockGetBootstrapData = getBootstrapData as jest.MockedFunction< + typeof getBootstrapData +>; + +beforeEach(() => { + jest.resetModules(); Review Comment: **Suggestion:** Using `jest.resetModules()` in `beforeEach` while holding on to `mockInitFeatureFlags` and `mockGetBootstrapData` created from the original imports means that after each reset the dynamically imported `./initEmbedded` will call *new* mocked functions, so the test-configured mocks are never invoked and your expectations on call counts/arguments will be checking the wrong mock instances. Re-bind the mock references after each `resetModules` so that the test uses the same mocked functions that `initEmbedded` imports in that run. [logic error] <details> <summary><b>Severity Level:</b> Major ⚠️</summary> ```mdx - ❌ Embedded feature flag unit tests failing. - ⚠️ CI test job may be unstable or fail. - ⚠️ False negatives hide real regressions. ``` </details> ```suggestion let mockInitFeatureFlags: jest.MockedFunction<typeof initFeatureFlags>; let mockGetBootstrapData: jest.MockedFunction<typeof getBootstrapData>; beforeEach(() => { jest.resetModules(); const core = jest.requireMock('@superset-ui/core') as { initFeatureFlags: typeof initFeatureFlags; }; const bootstrap = jest.requireMock('src/utils/getBootstrapData') as { default: typeof getBootstrapData; }; mockInitFeatureFlags = core.initFeatureFlags as jest.MockedFunction< typeof initFeatureFlags >; mockGetBootstrapData = bootstrap.default as jest.MockedFunction< typeof getBootstrapData >; ``` <details> <summary><b>Steps of Reproduction ✅ </b></summary> ```mdx 1. Open file superset-frontend/src/embedded/initEmbedded.test.ts. The top-level imports are at lines 19-20 (`import { initFeatureFlags } from '@superset-ui/core';` and `import getBootstrapData from 'src/utils/getBootstrapData';`). 2. jest.mock factories for both modules are declared at lines 22-30, so the test file provides mocked implementations before any requires. 3. At lines 32-37 the test binds consts: `mockInitFeatureFlags` and `mockGetBootstrapData` to the imported symbols (these are evaluated once when the test module is loaded). 4. The beforeEach (lines 39-43) calls `jest.resetModules()` which clears Node's module registry; subsequent dynamic imports (step 5) will load fresh module instances and fresh mock functions created by the module factories. 5. Each test does a dynamic import at line 58 (`const { bootstrapData } = await import('./initEmbedded');`) which causes the freshly reset mocked modules to be required by the imported module. Those newly created mock functions are different objects than the original `mockInitFeatureFlags` and `mockGetBootstrapData` variables bound at module load time. 6. The test's assertions at lines 60-66 (`expect(mockGetBootstrapData).toHaveBeenCalledTimes(1);` and `expect(mockInitFeatureFlags).toHaveBeenCalledTimes(1);`) therefore inspect the stale mock references bound at step 3, not the fresh mocks used by the dynamically imported `initEmbedded` module, causing the expectations to be incorrect and tests to fail or be flaky. 7. Reproducing locally: run `jest superset-frontend/src/embedded/initEmbedded.test.ts` (or full test suite). Observe failing or unexpected assertion results for call counts/arguments due to mock reference mismatch as described. ``` </details> <details> <summary><b>Prompt for AI Agent 🤖 </b></summary> ```mdx This is a comment left during a code review. **Path:** superset-frontend/src/embedded/initEmbedded.test.ts **Line:** 32:40 **Comment:** *Logic Error: Using `jest.resetModules()` in `beforeEach` while holding on to `mockInitFeatureFlags` and `mockGetBootstrapData` created from the original imports means that after each reset the dynamically imported `./initEmbedded` will call *new* mocked functions, so the test-configured mocks are never invoked and your expectations on call counts/arguments will be checking the wrong mock instances. Re-bind the mock references after each `resetModules` so that the test uses the same mocked functions that `initEmbedded` imports in that run. Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise. ``` </details> -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
