Copilot commented on code in PR #38033:
URL: https://github.com/apache/superset/pull/38033#discussion_r2874011032
##########
superset-frontend/src/pages/Login/Login.test.tsx:
##########
@@ -17,21 +17,45 @@
* under the License.
*/
import { render, screen } from 'spec/helpers/testing-library';
+import getBootstrapData from 'src/utils/getBootstrapData';
import Login from './index';
jest.mock('src/utils/getBootstrapData', () => ({
__esModule: true,
- default: () => ({
+ default: jest.fn(() => ({
common: {
conf: {
AUTH_TYPE: 1,
AUTH_PROVIDERS: [],
AUTH_USER_REGISTRATION: false,
},
},
- }),
+ })),
}));
+const mockMakeUrl = jest.fn((path: string) => path);
+jest.mock('src/utils/pathUtils', () => ({
+ makeUrl: (...args: string[]) => mockMakeUrl(...args),
Review Comment:
The test suite mocks `src/utils/pathUtils` to only provide `makeUrl`, but
`Login` now imports and calls `ensureAppRoot`. With the current mock,
`ensureAppRoot` will be `undefined` at runtime and the tests will fail. Update
the mock to export `ensureAppRoot` (and/or stop mocking `pathUtils` and set up
bootstrap data so the real `ensureAppRoot` works).
```suggestion
const mockMakeUrl = jest.fn((path: string) => path);
const mockEnsureAppRoot = jest.fn((path: string) => path);
jest.mock('src/utils/pathUtils', () => ({
makeUrl: (...args: string[]) => mockMakeUrl(...args),
ensureAppRoot: (path: string) => mockEnsureAppRoot(path),
```
##########
superset-frontend/src/pages/Login/Login.test.tsx:
##########
@@ -53,3 +77,232 @@ test('should render form instruction text', () => {
screen.getByText('Enter your login and password below:'),
).toBeInTheDocument();
});
+
+test('should call makeUrl for login endpoint on DB auth', () => {
+ render(<Login />, { useRedux: true });
+ expect(mockMakeUrl).toHaveBeenCalledWith('/login/');
+});
Review Comment:
This assertion (and several others below) expects `makeUrl` to be called for
`/login/...` URLs, but `Login` no longer uses `makeUrl` (it uses
`ensureAppRoot` directly). The expectations should be updated to match the new
behavior (e.g., assert on the rendered `href`/endpoint after prefixing, or spy
on `ensureAppRoot`).
--
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]
