Merge pull request #2547 from spotify/freben/register

chore(register-component): just a bunch of cleanup of code and tests

plugins/register-component/src/components/RegisterComponentForm/RegisterComponentForm.test.tsx

@@ -14,57 +14,38 @@
  * limitations under the License.
  */
 
-import { cleanup, fireEvent, render } from '@testing-library/react';
+import { act, render, screen } from '@testing-library/react';
+import userEvent from '@testing-library/user-event';
 import React from 'react';
-import { act } from 'react-dom/test-utils';
-import RegisterComponentForm, { Props } from './RegisterComponentForm';
+import { RegisterComponentForm } from './RegisterComponentForm';
 
-const setup = (props?: Partial<Props>) => {
-  return {
-    rendered: render(
-      <RegisterComponentForm
-        onSubmit={jest.fn()}
-        submitting={false}
-        {...props}
-      />,
-    ),
-  };
-};
 describe('RegisterComponentForm', () => {
-  afterEach(() => cleanup());
-
   it('should initially render a disabled button', async () => {
-    const { rendered } = setup();
+    render(<RegisterComponentForm onSubmit={jest.fn()} />);
+
     expect(
-      await rendered.findByText(
-        'Enter the full path to the component.yaml file in GitHub, GitLab, Bitbucket or Azure to start tracking your component.',
-      ),
+      await screen.findByText(/Enter the full path to the catalog-info.yaml/),
     ).toBeInTheDocument();
 
-    const submit = (await rendered.getByTestId(
-      'registerComponentFormSubmit',
-    )) as HTMLButtonElement;
-    expect(submit.disabled).toBeTruthy();
+    expect(screen.getByText('Submit').closest('button')).toBeDisabled();
   });
 
-  it('should enable a submit form when data when component url is set ', async () => {
-    const { rendered } = setup();
-    const input = (await rendered.getByRole('textbox')) as HTMLInputElement;
+  it('should enable a submit button when the target url is set ', async () => {
+    render(<RegisterComponentForm onSubmit={jest.fn()} />);
+
     await act(async () => {
-      // react-hook-form uses `input` event for changes
-      fireEvent.input(input, {
-        target: { value: 'https://example.com/blob/master/component.yaml' },
-      });
+      await userEvent.type(
+        await screen.findByLabelText('Entity file URL', { exact: false }),
+        'https://example.com/blob/master/component.yaml',
+      );
     });
-    const submit = (await rendered.getByTestId(
-      'registerComponentFormSubmit',
-    )) as HTMLButtonElement;
 
-    expect(submit.disabled).toBeFalsy();
+    expect(screen.getByText('Submit').closest('button')).not.toBeDisabled();
+  });
+
+  it('should show spinner while submitting', async () => {
+    render(<RegisterComponentForm onSubmit={jest.fn()} submitting />);
+
+    expect(screen.getByTestId('loading-progress')).toBeInTheDocument();
   });
 });
-
-it('should show spinner while submitting', async () => {
-  const { rendered } = setup({ submitting: true });
-  expect(rendered.getByTestId('loading-progress')).toBeInTheDocument();
-});

plugins/register-component/src/components/RegisterComponentForm/RegisterComponentForm.tsx

@@ -19,15 +19,15 @@ import {
   Button,
   FormControl,
   FormHelperText,
-  LinearProgress,
-  TextField,
-  Select,
-  MenuItem,
   InputLabel,
+  LinearProgress,
+  MenuItem,
+  Select,
+  TextField,
 } from '@material-ui/core';
 import { makeStyles } from '@material-ui/core/styles';
-import React, { FC } from 'react';
-import { useForm, Controller } from 'react-hook-form';
+import React from 'react';
+import { Controller, useForm } from 'react-hook-form';
 import { ComponentIdValidators } from '../../util/validate';
 
 const useStyles = makeStyles<BackstageTheme>(theme => ({
@@ -39,14 +39,17 @@ const useStyles = makeStyles<BackstageTheme>(theme => ({
   submit: {
     marginTop: theme.spacing(1),
   },
+  select: {
+    minWidth: 120,
+  },
 }));
 
 export type Props = {
   onSubmit: (formData: Record<string, string>) => Promise<void>;
-  submitting: boolean;
+  submitting?: boolean;
 };
 
-const RegisterComponentForm: FC<Props> = ({ onSubmit, submitting }) => {
+export const RegisterComponentForm = ({ onSubmit, submitting }: Props) => {
   const { control, register, handleSubmit, errors, formState } = useForm({
     mode: 'onChange',
   });
@@ -67,14 +70,14 @@ const RegisterComponentForm: FC<Props> = ({ onSubmit, submitting }) => {
         <TextField
           id="registerComponentInput"
           variant="outlined"
-          label="Component file URL"
+          label="Entity file URL"
           data-testid="componentLocationInput"
           error={hasErrors}
-          placeholder="https://example.com/user/some-service/blob/master/component.yaml"
+          placeholder="https://example.com/user/some-service/blob/master/catalog-info.yaml"
           name="componentLocation"
           required
           margin="normal"
-          helperText="Enter the full path to the component.yaml file in GitHub, GitLab, Bitbucket or Azure to start tracking your component."
+          helperText="Enter the full path to the catalog-info.yaml file in GitHub, GitLab, Bitbucket or Azure to start tracking your component."
           inputRef={register({
             required: true,
             validate: ComponentIdValidators,
@@ -88,8 +91,8 @@ const RegisterComponentForm: FC<Props> = ({ onSubmit, submitting }) => {
         )}
       </FormControl>
 
-      <FormControl variant="outlined">
-        <InputLabel id="scmLabel">SCM Detection</InputLabel>
+      <FormControl variant="outlined" className={classes.select}>
+        <InputLabel id="scmLabel">Host type</InputLabel>
         <Controller
           control={control}
           name="scmType"
@@ -112,8 +115,6 @@ const RegisterComponentForm: FC<Props> = ({ onSubmit, submitting }) => {
         />
       </FormControl>
       <Button
-        id="registerComponentFormSubmit"
-        data-testid="registerComponentFormSubmit"
         variant="contained"
         color="primary"
         type="submit"
@@ -125,5 +126,3 @@ const RegisterComponentForm: FC<Props> = ({ onSubmit, submitting }) => {
     </form>
   );
 };
-
-export default RegisterComponentForm;

plugins/register-component/src/components/RegisterComponentForm/index.ts

@@ -14,4 +14,4 @@
  * limitations under the License.
  */
 
-export { default } from './RegisterComponentForm';
+export { RegisterComponentForm } from './RegisterComponentForm';

plugins/register-component/src/components/RegisterComponentPage/RegisterComponentPage.test.tsx

@@ -14,21 +14,24 @@
  * limitations under the License.
  */
 
-import React from 'react';
-import { render, cleanup } from '@testing-library/react';
-import { RegisterComponentPage } from './RegisterComponentPage';
-import { ThemeProvider } from '@material-ui/core';
-import { lightTheme } from '@backstage/theme';
 import {
-  errorApiRef,
   ApiProvider,
   ApiRegistry,
   createRouteRef,
+  errorApiRef,
 } from '@backstage/core';
 import { catalogApiRef } from '@backstage/plugin-catalog';
+import { lightTheme } from '@backstage/theme';
+import { ThemeProvider } from '@material-ui/core';
+import { render, screen } from '@testing-library/react';
+import React from 'react';
 import { MemoryRouter } from 'react-router-dom';
+import { RegisterComponentPage } from './RegisterComponentPage';
 
-const errorApi = { post: () => {} };
+const errorApi: jest.Mocked<typeof errorApiRef.T> = {
+  post: jest.fn(),
+  error$: jest.fn(),
+};
 
 const catalogApi: jest.Mocked<typeof catalogApiRef.T> = {
   /* eslint-disable-next-line @typescript-eslint/no-unused-vars */
@@ -40,35 +43,31 @@ const catalogApi: jest.Mocked<typeof catalogApiRef.T> = {
   getEntityByName: jest.fn(),
 };
 
-const setup = () => ({
-  rendered: render(
-    <MemoryRouter>
-      <ApiProvider
-        apis={ApiRegistry.from([
-          [errorApiRef, errorApi],
-          [catalogApiRef, catalogApi],
-        ])}
-      >
-        <ThemeProvider theme={lightTheme}>
-          <RegisterComponentPage
-            catalogRouteRef={createRouteRef({
-              path: '/catalog',
-              title: 'Service Catalog',
-            })}
-          />
-        </ThemeProvider>
-      </ApiProvider>
-    </MemoryRouter>,
-  ),
-});
+const Wrapper = ({ children }: { children?: React.ReactNode }) => (
+  <MemoryRouter>
+    <ApiProvider
+      apis={ApiRegistry.with(errorApiRef, errorApi).with(
+        catalogApiRef,
+        catalogApi,
+      )}
+    >
+      <ThemeProvider theme={lightTheme}>{children}</ThemeProvider>
+    </ApiProvider>
+  </MemoryRouter>
+);
 
 describe('RegisterComponentPage', () => {
-  afterEach(() => cleanup());
-
   it('should render', () => {
-    const { rendered } = setup();
-    expect(
-      rendered.getByText('Register existing component'),
-    ).toBeInTheDocument();
+    render(
+      <RegisterComponentPage
+        catalogRouteRef={createRouteRef({
+          path: '/catalog',
+          title: 'Service Catalog',
+        })}
+      />,
+      { wrapper: Wrapper },
+    );
+
+    expect(screen.getByText('Register existing component')).toBeInTheDocument();
   });
 });

plugins/register-component/src/components/RegisterComponentPage/RegisterComponentPage.tsx

@@ -28,7 +28,7 @@ import {
   ContentHeader,
   RouteRef,
 } from '@backstage/core';
-import RegisterComponentForm from '../RegisterComponentForm';
+import { RegisterComponentForm } from '../RegisterComponentForm';
 import { catalogApiRef } from '@backstage/plugin-catalog';
 import { useMountedState } from 'react-use';
 import { Entity, Location } from '@backstage/catalog-model';
@@ -55,6 +55,7 @@ const FormStates = {
 } as const;
 
 type ValuesOf<T> = T extends Record<any, infer V> ? V : never;
+
 export const RegisterComponentPage = ({
   catalogRouteRef,
 }: {
@@ -85,9 +86,9 @@ export const RegisterComponentPage = ({
     const { scmType, componentLocation: target } = formData;
     try {
       const typeMapping = [
-        { url: /https:\/\/gitlab\.com\/.*/, type: 'gitlab' },
-        { url: /https:\/\/bitbucket\.org\/.*/, type: 'bitbucket/api' },
-        { url: /https:\/\/dev\.azure\.com\/.*/, type: 'azure/api' },
+        { url: /^https:\/\/gitlab\.com\/.*/, type: 'gitlab' },
+        { url: /^https:\/\/bitbucket\.org\/.*/, type: 'bitbucket/api' },
+        { url: /^https:\/\/dev\.azure\.com\/.*/, type: 'azure/api' },
         { url: /.*/, type: 'github' },
       ];
 

plugins/register-component/src/components/RegisterComponentResultDialog/RegisterComponentResultDialog.test.tsx

@@ -14,47 +14,40 @@
  * limitations under the License.
  */
 
+import { Entity } from '@backstage/catalog-model';
+import { createRouteRef } from '@backstage/core';
 import { lightTheme } from '@backstage/theme';
 import { ThemeProvider } from '@material-ui/core';
-import { cleanup, render } from '@testing-library/react';
-import React, { ComponentProps } from 'react';
+import { render, screen } from '@testing-library/react';
+import React from 'react';
 import { MemoryRouter } from 'react-router-dom';
 import { RegisterComponentResultDialog } from './RegisterComponentResultDialog';
-import { createRouteRef } from '@backstage/core';
 
-const setup = (
-  props?: Partial<ComponentProps<typeof RegisterComponentResultDialog>>,
-) => ({
-  rendered: render(
-    <MemoryRouter>
-      <ThemeProvider theme={lightTheme}>
-        <RegisterComponentResultDialog
-          onClose={() => {}}
-          entities={[]}
-          catalogRouteRef={createRouteRef({
-            path: '/catalog',
-            title: 'Service Catalog',
-          })}
-          {...props}
-        />
-      </ThemeProvider>
-    </MemoryRouter>,
-  ),
-});
+const Wrapper = ({ children }: { children?: React.ReactNode }) => (
+  <MemoryRouter>
+    <ThemeProvider theme={lightTheme}>{children}</ThemeProvider>
+  </MemoryRouter>
+);
+
 describe('RegisterComponentResultDialog', () => {
-  afterEach(() => cleanup());
-
   it('should render', () => {
-    const { rendered } = setup();
-    expect(
-      rendered.getByText('Component Registration Result'),
-    ).toBeInTheDocument();
-  });
-});
+    render(
+      <RegisterComponentResultDialog
+        onClose={() => {}}
+        entities={[]}
+        catalogRouteRef={createRouteRef({
+          path: '/catalog',
+          title: 'Service Catalog',
+        })}
+      />,
+      { wrapper: Wrapper },
+    );
 
-it('should show a list of components if success', async () => {
-  const { rendered } = setup({
-    entities: [
+    expect(screen.getByText('Registration Result')).toBeInTheDocument();
+  });
+
+  it('should show a list of components if success', async () => {
+    const entities: Entity[] = [
       {
         apiVersion: 'backstage.io/v1alpha1',
         kind: 'Component',
@@ -75,14 +68,26 @@ it('should show a list of components if success', async () => {
           type: 'service',
         },
       },
-    ],
-  });
+    ];
 
-  expect(
-    rendered.getByText(
-      'The following components have been succefully created:',
-    ),
-  ).toBeInTheDocument();
-  expect(rendered.getByText('Component1')).toBeInTheDocument();
-  expect(rendered.getByText('Component2')).toBeInTheDocument();
+    render(
+      <RegisterComponentResultDialog
+        onClose={() => {}}
+        entities={entities}
+        catalogRouteRef={createRouteRef({
+          path: '/catalog',
+          title: 'Service Catalog',
+        })}
+      />,
+      { wrapper: Wrapper },
+    );
+
+    expect(
+      screen.getByText(
+        'The following entities have been successfully created:',
+      ),
+    ).toBeInTheDocument();
+    expect(screen.getByText('Component1')).toBeInTheDocument();
+    expect(screen.getByText('Component2')).toBeInTheDocument();
+  });
 });

plugins/register-component/src/components/RegisterComponentResultDialog/RegisterComponentResultDialog.tsx

@@ -73,10 +73,10 @@ export const RegisterComponentResultDialog = ({
   catalogRouteRef,
 }: Props) => (
   <Dialog open onClose={onClose} classes={classes}>
-    <DialogTitle>Component Registration Result</DialogTitle>
+    <DialogTitle>Registration Result</DialogTitle>
     <DialogContent>
       <DialogContentText>
-        The following components have been succefully created:
+        The following entities have been successfully created:
       </DialogContentText>
       <List>
         {entities.map((entity: any, index: number) => {