662/asterner profile tests #673

AJSterner · 2024-08-28T16:57:44Z

This PR:

Resolves (part of) #662

1. Adds basic render tests for the Profile page
2. Adds tests to check basic functionality for the Profile page
3. Factors out a MockSignedInUserContext to allow multiple tests to access user profiles

Future Steps:

We should figure out why there's a second timeout before loading the full profile page...

PASS/src/pages/Profile.jsx

Lines 114 to 128 in 52c881f

    
           useEffect(() => { 
        
             setTimeout(() => { 
        
               setLoadingProfile(false); 
        
             }, 1000); 
        
           }, []); 
        
           const truncatedText = selectedDocToDelete?.name ? truncateText(selectedDocToDelete.name) : ''; 
        
           if (loadingProfile) { 
        
             return ( 
        
               <LoadingAnimation loadingItem="Profile"> 
        
                 <CircularProgress /> 
        
               </LoadingAnimation> 
        
             ); 
        
           }

AJSterner · 2024-08-28T17:34:15Z

src/pages/Profile.jsx

@@ -145,7 +145,9 @@ const Profile = () => {
          width: isSmallScreen ? '100%' : 'auto'
        }}
      >
-        <Typography sx={{ fontWeight: 'bold', fontSize: '18px' }}>My Profile</Typography>
+        <Typography variant="h1" sx={{ fontWeight: 'bold', fontSize: '18px' }}>


This change makes this text into a proper heading without modifying the visual rendering

Looking over it, I'm not certain whether we need it to say "My Profile" at the top at all. The breadcrumb basically covers the same role already and we don't really apply this same concept to other pages (such as the Contacts page). But that's a different discussion

Agreed. I attempted to make minimal changes to the page itself. This seems like something we can take a look at post MVP.

SignedInUserContext needs to be mocked in multiple places. This commit adds a MockSignedInUserContext that can be used for this purpose.

andycwilliams

A couple of minor edits I'd like but it's looking good to me

andycwilliams · 2024-08-28T19:56:47Z

src/pages/Profile.jsx

@@ -145,7 +145,9 @@ const Profile = () => {
          width: isSmallScreen ? '100%' : 'auto'
        }}
      >
-        <Typography sx={{ fontWeight: 'bold', fontSize: '18px' }}>My Profile</Typography>
+        <Typography variant="h1" sx={{ fontWeight: 'bold', fontSize: '18px' }}>


Looking over it, I'm not certain whether we need it to say "My Profile" at the top at all. The breadcrumb basically covers the same role already and we don't really apply this same concept to other pages (such as the Contacts page). But that's a different discussion

andycwilliams · 2024-08-28T19:57:25Z

test/pages/Profile.test.jsx

+import { render, screen, cleanup, waitForElementToBeRemoved } from '@testing-library/react';
+import userEvent from '@testing-library/user-event';
+import { describe, expect, it, beforeEach, afterEach } from 'vitest';
+


Can delete these extra lines

andycwilliams · 2024-08-28T19:59:17Z

test/mocks/contexts/MockSignedInUserContext.jsx

+import { vi } from 'vitest';
+import { SignedInUserContext } from '@contexts';
+import * as profileHelper from '../../../src/model-helpers/Profile';
+


If we're going to keep this, we can leave a TODO note here to import it elsewhere. I'm pretty sure there are other tests that can make use of this but not as part of this PR

What would you like to add as a TODO?

I don't see the SignedInUserContext used in other tests (other than the tests for the context itself).

I'll put a pin in that idea. I want to think it over more

andycwilliams

There's one extra line that can be removed but approving anyway.

Also would like @leekahung to give this a once-over just to be extra safe

andycwilliams · 2024-08-30T00:31:39Z

test/mocks/contexts/MockSignedInUserContext.jsx

+import { vi } from 'vitest';
+import { SignedInUserContext } from '@contexts';
+import * as profileHelper from '../../../src/model-helpers/Profile';
+


I'll put a pin in that idea. I want to think it over more

test/components/Profile/ProfileComponent.test.jsx

AJSterner · 2024-08-30T00:45:08Z

There's one extra line that can be removed but approving anyway.

Also would like @leekahung to give this a once-over just to be extra safe

Sweet! I'll wait for feedback from @leekahung

leekahung

Just a comment on the mock context, but it's a positive one. I can approve this. 👍

leekahung · 2024-08-30T00:54:48Z

test/mocks/contexts/MockSignedInUserContext.jsx

@@ -0,0 +1,32 @@
+import React from 'react';


I see we're making mock contexts for the tests. I'm fine with this since it'll be useful in case multiple tests need the same context.

AJSterner · 2024-08-30T01:51:43Z

Awesome! Thanks y'all :) merged!

AJSterner marked this pull request as draft August 28, 2024 16:57

AJSterner commented Aug 28, 2024

View reviewed changes

AJSterner force-pushed the 662/asterner-profile-tests branch 2 times, most recently from 1005f5b to 3d9658e Compare August 28, 2024 17:41

AJSterner requested review from leekahung and andycwilliams August 28, 2024 17:55

AJSterner marked this pull request as ready for review August 28, 2024 17:57

AJSterner mentioned this pull request Aug 28, 2024

[Bug Report] - Fix Failing Tests #662

Closed

12 tasks

Andrew Sterner added 10 commits August 28, 2024 11:26

vite.config.js: add model-helpers alias

807a51a

Profile.test.jsx: initial render test

cd77d37

Profile test: fetchProfileInfo is not a function

d319b8f

Basic render and button tests work

0033d80

Basic render and button tests work

dce4146

remove model-helpers alias

a8fea76

Profile tests: mock out SignedInUserContext

dc17215

SignedInUserContext needs to be mocked in multiple places. This commit adds a MockSignedInUserContext that can be used for this purpose.

fix vite config formatting

01f2c44

Remove unused import

503ccd9

Fix prettier errors

1f8628a

AJSterner force-pushed the 662/asterner-profile-tests branch from b824b66 to 1f8628a Compare August 28, 2024 18:26

andycwilliams reviewed Aug 28, 2024

View reviewed changes

remove unnecessary blank lines

b146047

AJSterner requested a review from andycwilliams August 29, 2024 21:14

andycwilliams approved these changes Aug 30, 2024

View reviewed changes

another one another one

a7b7984

leekahung approved these changes Aug 30, 2024

View reviewed changes

AJSterner merged commit fb5f35d into Development Aug 30, 2024
3 checks passed

AJSterner deleted the 662/asterner-profile-tests branch August 30, 2024 04:32

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

662/asterner profile tests #673

662/asterner profile tests #673

AJSterner commented Aug 28, 2024 •

edited

Loading

AJSterner Aug 28, 2024

andycwilliams Aug 28, 2024

AJSterner Aug 29, 2024

andycwilliams left a comment

andycwilliams Aug 28, 2024

andycwilliams Aug 28, 2024

AJSterner Aug 29, 2024

andycwilliams Aug 28, 2024

AJSterner Aug 29, 2024

andycwilliams Aug 30, 2024

andycwilliams left a comment

andycwilliams Aug 30, 2024

AJSterner commented Aug 30, 2024

leekahung left a comment

leekahung Aug 30, 2024

AJSterner commented Aug 30, 2024

	useEffect(() => {
	setTimeout(() => {
	setLoadingProfile(false);
	}, 1000);
	}, []);

	const truncatedText = selectedDocToDelete?.name ? truncateText(selectedDocToDelete.name) : '';

	if (loadingProfile) {
	return (
	<LoadingAnimation loadingItem="Profile">
	<CircularProgress />
	</LoadingAnimation>
	);
	}

662/asterner profile tests #673

662/asterner profile tests #673

Conversation

AJSterner commented Aug 28, 2024 • edited Loading

This PR:

Future Steps:

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

andycwilliams left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

andycwilliams left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AJSterner commented Aug 30, 2024

leekahung left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AJSterner commented Aug 30, 2024

AJSterner commented Aug 28, 2024 •

edited

Loading