Update tests, Take 82#382
Conversation
|
this should probably be a draft PR, no? |
|
weird, the first issue i'm seeing is Cannot find module '@hylo/presenters/GroupPresenter' from 'src/util/navigation.js' |
|
oh i seem you fixed it :) |
|
it looks to me like babel is not handling the typescript files right. or maybe we need to add typescript support to jest... |
|
pushed fix for the import issue! |
|
oops, never mind, i thought it was working, but still broken |
|
ok for real fixed the import and luxon issues! |
Some ai assistance, I tried to keep mocks reasonable but it really helped figure out tough bugs in the tests
when running, they seem to spawn child threads that persist like zombies and never complete. Something to do with useState setters causing problems. /shrug
|
Okay, I've gone as far as I can -- @tibetsprague can you take this over the finish line? IF NOT -- PLEASE review a couple of tests, they point to some potential bugs:
These on my machine seem to run infinite. I tracked it down to an interaction between useState and useEffect: when any set function is call in a useEffect hook, it goes into some weird place and never returns. Odd. Really odd because in TopicSelector, if I comment out all the code of the second useEffect and only set the one boolean Also -- and this should be quick to fix -- check the these tests, bc either the tests are wrong or the app is wrong. I suspect the tests but I didn't have time to sleuth the answer:
|
Search doesnt make as many calls to the server and works faster and better
|
@tibetsprague what would you like me to do with this PR? |
| /> | ||
| <Tooltip | ||
| delay={550} | ||
| id={`pinned-post-tt-${id}`} |
There was a problem hiding this comment.
why was this added? is it it used anywhere?
| className='border-2 border-foreground/30 bg-foreground/30 px-2 py-1 rounded flex items-center' | ||
| dataTipHtml={!valid ? invalidMessage : ''} | ||
| dataFor='submit-tt' | ||
| label='submit' |
There was a problem hiding this comment.
this changes the text of the submit button? i dont think we want this
| if (centerColumn) centerColumn.scrollTop = 0 | ||
| }, [pathMatchParams?.context, pathMatchParams?.groupSlug, pathMatchParams?.view]) | ||
|
|
||
| /* First time viewing a group redirect to welcome page if it exists, otherwise home view */ |
There was a problem hiding this comment.
why is this being added on this branch? maybe a merge issue?
| function RolesSettingsTab ({ group, commonRoles }) { | ||
| const dispatch = useDispatch() | ||
| const suggestions = useSelector(state => state.RoleSettings.map(personId => getPerson(state, { personId }))) | ||
| const suggestions = createSelector([state => state.RoleSettings], state => state.RoleSettings.map(personId => getPerson(state, { personId }))) |
There was a problem hiding this comment.
why this change? we dont use this pattern in other places really
| } | ||
|
|
||
| function AddMemberToRole ({ | ||
| export function AddMemberToRole ({ |
There was a problem hiding this comment.
ideally we only export one component per file, so that the hot module reloading works. so we should move these into their own files to test them
| const fetchSearchResultsAction = () => { | ||
| return fetchSearchResultsDebounced({ search: searchForInput || searchFromQueryString, filter }) | ||
| } | ||
| const updateQueryParam = useCallback( |
There was a problem hiding this comment.
did i make these changes? 🤪
|
@KevinTriplett im down to take it to the finish line, unless you want to! |
|
Yeah, you should take it over -- I think all that weirdness happened when I merged dev into the branch. Sorry about that, maybe you can roll the merge back? |
|
Should I delete this PR and redo getting tests working? If I do, I'd vote to do a PR in chunks, like critical / obvious first and then questionable last, everything else in between? |
|
No lots of good work was done here. I’ve been intending to finish this one in the next week. But happy to have you jump back on it too!On Jun 14, 2025, at 1:37 PM, 'Kevin Triplett' via Developers ***@***.***> wrote:KevinTriplett left a comment (Hylozoic/hylo#382)
Should I delete this PR and redo getting tests working? If I do, I'd vote to do a PR in chunks, like critical / obvious first and then questionable last, everything else in between?
—Reply to this email directly, view it on GitHub, or unsubscribe.You are receiving this because you are subscribed to this thread.Message ID: ***@***.***>
|
3 participants
Per title, my first pass at updating tests
I need help with these two errors, which affect many files.
Error 1: surely, you jest!
SyntaxError: Cannot use import statement outside a moduleThis seems to be related to jest and the babel/preset-env module. Here's a SO post
Error 2: DateTime oddity
Cannot read properties of undefined (reading 'indexOf')For some reason, luxon's DateTime is throwing this error on
DateTime.fromISO(post.createdAt)even when createdAt is a valid ISO string. I tried to gather clues for this one but no luck. A right head scratcher.Other errors, I'm not qualified to touch:
I did touch a few, but feel they need a review for accuracy / intent. Like:
process.env.VITE_PROXY_HOSTvsprocess.env.PROXY_HOSTOthers I didn't want to touch, thank you:
MessageThread › marks a thread as unread if lastReadAt is older than updatedAtI'll try to find other low hanging fruit, but I'll need help on Errors 1 and 2. Once these are solved, it should help a great deal.