Improve testing posture (#40)

* fix linting

* update testing

* add better unit testing

* enable corepack

* fix corepack enable

* remove unit testing from post-deployment pipeline

* update names

* get s3 client directly

.github/workflows/deploy-dev.yml

@@ -7,12 +7,30 @@ on:
     branches:
       - main
 jobs:
+  test-unit:
+    runs-on: ubuntu-latest
+    name: Run Unit Tests
+    steps:
+      - uses: actions/checkout@v3
+      - name: Set up Python 3.11 for testing
+        uses: actions/setup-python@v5
+        with:
+          python-version: 3.11
+      - name: Setup Node LTS
+        uses: actions/setup-node@v4
+        with:
+          node-version: 18
+      - name: Run unit testing
+        run: make test_unit
   deploy:
     runs-on: ubuntu-latest
     concurrency:
       group: resume-book-aws-dev
       cancel-in-progress: false
     environment: "AWS DEV"
+    name: Deploy to AWS DEV
+    needs:
+      - test-unit
     steps:
       - uses: actions/checkout@v3
       - uses: aws-actions/setup-sam@v2
@@ -32,6 +50,8 @@ jobs:
     permissions:
       contents: read
       deployments: write  
+    needs:
+      - test-unit
     name: Deploy to Cloudflare Pages DEV
     environment: "Cloudflare Pages - Dev"
     steps:
@@ -58,6 +78,7 @@ jobs:
           gitHubToken: ${{ secrets.GITHUB_TOKEN }}
   test:
     runs-on: ubuntu-latest
+    name: Run Live Integration Tests
     needs:
       - deploy
       - deploy-cf-pages-dev
@@ -67,7 +88,11 @@ jobs:
         uses: actions/setup-python@v5
         with:
           python-version: 3.11
+      - name: Setup Node LTS
+        uses: actions/setup-node@v4
+        with:
+          node-version: 18
       - name: Run live testing
-        run: make test_ci
+        run: make test_live_integration
         env:
           RB_JWT_SECRET: ${{ secrets.RB_JWT_SECRET }}

.github/workflows/deploy.yml

@@ -7,8 +7,25 @@ on:
     branches:
       - main
 jobs:
+  test-unit:
+    runs-on: ubuntu-latest
+    name: Run Unit Tests
+    steps:
+      - uses: actions/checkout@v3
+      - name: Set up Python 3.11 for testing
+        uses: actions/setup-python@v5
+        with:
+          python-version: 3.11
+      - name: Setup Node LTS
+        uses: actions/setup-node@v4
+        with:
+          node-version: 18
+      - name: Run unit testing
+        run: make test_unit
   deploy-aws-dev:
     runs-on: ubuntu-latest
+    needs:
+      - test-unit
     concurrency:
       group: resume-book-aws-dev
       cancel-in-progress: false
@@ -27,6 +44,8 @@ jobs:
       - run: make deploy_dev
   deploy-cf-pages-dev:
     runs-on: ubuntu-latest
+    needs:
+      - test-unit
     concurrency:
       group: resume-book-cf-pages-dev
       cancel-in-progress: false
@@ -69,8 +88,12 @@ jobs:
         uses: actions/setup-python@v5
         with:
           python-version: 3.11
-      - name: Run live testing
-        run: make test_ci
+      - name: Setup Node LTS
+        uses: actions/setup-node@v4
+        with:
+          node-version: 18
+      - name: Run integration testing
+        run: make test_live_integration
         env:
           RB_JWT_SECRET: ${{ secrets.RB_JWT_SECRET }}
   deploy-aws-prod:

Makefile

@@ -27,9 +27,19 @@ deploy_prod: check_account_prod build
 deploy_dev: check_account_dev build
 	sam deploy $(common_params) --parameter-overrides $(run_env)=dev
 
-test_ci:
+install_deps_python:
 	pip install -r tests/live_integration/requirements.txt
+	pip install -r api/requirements-testing.txt
+
+install_deps_node:
+	cd clientv2 && corepack enable && yarn
+
+test_live_integration: install_deps_python
 	pytest -rP tests/live_integration/
 
+test_unit: install_deps_python install_deps_node
+	pytest -rP api/
+	cd clientv2 && yarn test
+
 generate_jwt:
 	python utils/generate_jwt.py

api/requirements-testing.txt

@@ -0,0 +1,2 @@
+pytest
+moto

code/util/s3.py → api/util/s3.py

@@ -2,13 +2,9 @@
 from util.logging import get_logger
 import boto3
 import re
-import os
-
-session = boto3.Session(region_name=os.environ.get('AWS_REGION', 'us-east-1'))
-s3_client = session.client('s3')
 logger = get_logger()
 
-def create_presigned_url_from_s3_url(s3_url, expiration=60):
+def create_presigned_url_from_s3_url(s3_client, s3_url, expiration=60):
     """
     Generate a presigned URL to share an S3 object
 
@@ -35,7 +31,7 @@ def create_presigned_url_from_s3_url(s3_url, expiration=60):
 
     return response
 
-def create_presigned_url_for_put(bucket_name, object_key, file_size, expiration=300):
+def create_presigned_url_for_put(s3_client, bucket_name, object_key, file_size, expiration=300):
     """
     Generate a presigned URL to upload an S3 object
 

code/util/server.py → api/util/server.py

@@ -43,6 +43,7 @@
 app = APIGatewayRestResolver(cors=cors_config)
 session = boto3.Session(region_name=os.environ.get("AWS_REGION", "us-east-1"))
 secretsmanager = session.client("secretsmanager")
+s3_client = session.client('s3')
 db_config = get_parameter_from_sm(secretsmanager, "infra-resume-book-db-config")
 openai_client = get_oai_client(db_config['oai_key'])
 
@@ -85,6 +86,7 @@ def shared_get_profile(username):
         )
     if profile_data and 'resumePdfUrl' in profile_data:
         profile_data["resumePdfUrl"] = create_presigned_url_from_s3_url(
+            s3_client,
             profile_data["resumePdfUrl"]
         )
     return Response(
@@ -206,6 +208,7 @@ def student_get_s3_presigned():
             },
         )
     presigned_url = create_presigned_url_for_put(
+        s3_client=s3_client,
         bucket_name=S3_BUCKET,
         object_key=f"resume_{username}.pdf",
         file_size=data["file_size"],
@@ -237,7 +240,7 @@ def student_gpt():
         for prop in url_properties:
             if response[prop] != "" and "http://" not in response[prop] and "https://" not in response[prop]:
                 response[prop] = f"http://{response[prop]}"
-        response['resumePdfUrl'] = create_presigned_url_from_s3_url(f"s3://{S3_BUCKET}/resume_{username}.pdf")
+        response['resumePdfUrl'] = create_presigned_url_from_s3_url(s3_client, f"s3://{S3_BUCKET}/resume_{username}.pdf")
     except pydantic.ValidationError as e:
         return Response(
             status_code=403,

api/util/tests/test_secretsmanager.py

@@ -0,0 +1,42 @@
+import pytest
+from moto import mock_aws
+import boto3
+import os
+import json
+from ..secretsmanager import get_parameter_from_sm
+
+
+key_data = {"CLIENT_DATA": "12345", "CLIENT_SECRET": "12345"}
+invalid_key_data = '{"name": "Joe", "age": null]'
+@pytest.fixture
+def sm_client():
+    """Fixture to create a mocked KMS client using moto."""
+    with mock_aws():
+        client = boto3.client('secretsmanager', region_name=os.environ.get("AWS_REGION", "us-east-1"))
+        yield client
+
+@pytest.fixture
+def sm_valid_key_id(sm_client):
+    """Fixture to create a mock KMS key."""
+    sm_client.create_secret(Name='test-secret', SecretString=json.dumps(key_data))
+    return 'test-secret'
+
+
+@pytest.fixture
+def sm_invalid_key_id(sm_client):
+    """Fixture to create a mock KMS key."""
+    sm_client.create_secret(Name='test-invalid-secret', SecretString=invalid_key_data)
+    return 'test-invalid-secret'
+
+def test_valid_secret(sm_client, sm_valid_key_id):
+    assert key_data == get_parameter_from_sm(sm_client, sm_valid_key_id)
+
+def test_invalid_secret(sm_client, sm_invalid_key_id, capfd):
+    assert get_parameter_from_sm(sm_client, sm_invalid_key_id) == None
+    out, _ = capfd.readouterr()
+    assert out == "Parameter \"test-invalid-secret\" is not in valid JSON format.\n"
+
+def test_nonexistent_secret(sm_client, capfd):
+    assert get_parameter_from_sm(sm_client, 'test-nonexistent-secret') == None
+    out, _ = capfd.readouterr()
+    assert out == "Parameter \"test-nonexistent-secret\" not found.\n"

clientv2/.eslintrc.cjs

@@ -6,5 +6,7 @@ module.exports = {
     rules: {
       'react/react-in-jsx-scope': 'off',
       'import/extensions': 'off',
+      'no-plusplus': [2, { allowForLoopAfterthoughts: true }],
+      'no-console': [2, { allow: ['warn', 'error'] }],
     },
   };

clientv2/src/Router.tsx

@@ -1,4 +1,4 @@
-import React, { useState, useEffect, ReactNode, ErrorInfo } from 'react';
+import React, { useState, useEffect, ReactNode } from 'react';
 import { createBrowserRouter, Navigate, RouterProvider } from 'react-router-dom';
 import { AuthRoleEnum, useAuth } from './components/AuthContext';
 import { LoginPage } from './pages/Login.page';
@@ -84,14 +84,14 @@ const ErrorBoundary: React.FC<ErrorBoundaryProps> = ({ children }) => {
   const [hasError, setHasError] = useState(false);
   const [error, setError] = useState<Error | null>(null);
 
-  const onError = (error: Error, errorInfo: ErrorInfo) => {
+  const onError = (errorObj: Error) => {
     setHasError(true);
-    setError(error);
+    setError(errorObj);
   };
 
   useEffect(() => {
     const errorHandler = (event: ErrorEvent) => {
-      onError(event.error, { componentStack: '' });
+      onError(event.error);
     };
     window.addEventListener('error', errorHandler);
 
@@ -104,7 +104,7 @@ const ErrorBoundary: React.FC<ErrorBoundaryProps> = ({ children }) => {
     if (error.message === '404') {
       return <Error404Page />;
     }
-    return <Error500Page />
+    return <Error500Page />;
   }
 
   return <>{children}</>;
@@ -113,11 +113,12 @@ const ErrorBoundary: React.FC<ErrorBoundaryProps> = ({ children }) => {
 export const Router: React.FC = () => {
   const { isLoggedIn, userData } = useAuth();
 
-  const router = !isLoggedIn || !userData
-    ? unauthenticatedRouter
-    : userData.role === AuthRoleEnum.RECRUITER
-      ? recruiterRouter
-      : studentRouter;
+  const router =
+    !isLoggedIn || !userData
+      ? unauthenticatedRouter
+      : userData.role === AuthRoleEnum.RECRUITER
+        ? recruiterRouter
+        : studentRouter;
 
   return (
     <ErrorBoundary>

clientv2/src/components/AuthContext/index.tsx

@@ -15,7 +15,6 @@ import {
 } from '@azure/msal-browser';
 import { MantineProvider } from '@mantine/core';
 import FullScreenLoader from './LoadingScreen';
-import { notifications } from '@mantine/notifications';
 
 export enum AuthSourceEnum {
   MSAL,
@@ -78,7 +77,9 @@ export const AuthProvider: React.FC<AuthProviderProps> = ({ children }) => {
 
   useEffect(() => {
     if (isAuthenticated && !isLoading && !userData) {
-      const isRecruiter = getKindePermission(`recruiter:resume-book-${import.meta.env.VITE_RUN_ENVIRONMENT}`).isGranted;
+      const isRecruiter = getKindePermission(
+        `recruiter:resume-book-${import.meta.env.VITE_RUN_ENVIRONMENT}`
+      ).isGranted;
       if (!isRecruiter) {
         setUserData(null);
         setIsLoggedIn(false);
@@ -102,7 +103,7 @@ export const AuthProvider: React.FC<AuthProviderProps> = ({ children }) => {
         handleMsalResponse(response);
       } else if (accounts.length > 0) {
         // User is already logged in, set the state
-        const [lastName, firstName] = accounts[0].name?.split(',')!;
+        const [lastName, firstName] = accounts[0].name?.split(',')! || [];
         setUserData({
           email: accounts[0].username,
           name: `${firstName} ${lastName}`,
@@ -175,9 +176,9 @@ export const AuthProvider: React.FC<AuthProviderProps> = ({ children }) => {
   }, [userData, instance, getKindeToken]);
 
   const loginMsal = useCallback(async () => {
-    const accounts = instance.getAllAccounts();
-    if (accounts.length > 0) {
-      instance.setActiveAccount(accounts[0]);
+    const accountsLocal = instance.getAllAccounts();
+    if (accountsLocal.length > 0) {
+      instance.setActiveAccount(accountsLocal[0]);
     } else {
       await instance.loginRedirect();
     }

clientv2/src/components/FullPageError/index.test.tsx

@@ -0,0 +1,28 @@
+import userEvent from '@testing-library/user-event';
+import { describe, it, expect, vi } from 'vitest';
+import { render, screen } from '@/test-utils';
+import FullPageError from './index'; // Adjust the import path as necessary
+
+describe('FullPageError', () => {
+  it('renders with default error messages when no props are provided', () => {
+    render(<FullPageError />);
+    expect(screen.getByText('An error occurred')).toBeInTheDocument();
+    expect(screen.getByText('Something went wrong. Please try again later.')).toBeInTheDocument();
+    expect(screen.queryByRole('button', { name: 'Retry' })).toBeNull();
+  });
+
+  it('renders custom error codes and messages when provided', () => {
+    render(<FullPageError errorCode={404} errorMessage="Page not found" />);
+    expect(screen.getByText('404')).toBeInTheDocument();
+    expect(screen.getByText('Page not found')).toBeInTheDocument();
+  });
+
+  it('displays a retry button when an onRetry handler is provided', async () => {
+    const onRetry = vi.fn();
+    render(<FullPageError onRetry={onRetry} />);
+    const retryButton = screen.getByTestId('errorRetryButton');
+    expect(retryButton).toBeInTheDocument();
+    await userEvent.click(retryButton);
+    expect(onRetry).toHaveBeenCalled();
+  });
+});

clientv2/src/components/FullPageError/index.tsx

@@ -13,7 +13,7 @@ const FullPageError: React.FC<FullPageErrorProps> = ({ errorCode, errorMessage,
       <Title>{errorCode || 'An error occurred'}</Title>
       <Text color="dimmed">{errorMessage || 'Something went wrong. Please try again later.'}</Text>
       {onRetry && (
-        <Button variant="outline" onClick={onRetry}>
+        <Button variant="outline" onClick={onRetry} data-testid="errorRetryButton">
           Retry
         </Button>
       )}

clientv2/src/components/LoginComponent/index.tsx

@@ -9,10 +9,10 @@ import {
   Anchor,
   Title,
 } from '@mantine/core';
+import { IconLock } from '@tabler/icons-react';
 import { AcmLoginButton } from './AcmLoginButton';
 import { PartnerLoginButton } from './PartnerLoginButton';
 import brandImgUrl from '@/banner-blue.png';
-import { IconLock } from '@tabler/icons-react';
 
 export function LoginComponent(props: PaperProps) {
   return (