-
Notifications
You must be signed in to change notification settings - Fork 124
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Tests for Tutorial #2702
Tests for Tutorial #2702
Changes from 21 commits
3f522c1
988f092
8496e3a
29d2790
34aa6df
46dcbd6
fb9ecdd
78dce26
098ff17
eedd501
16ff569
8db9c57
fa4dc17
49c3e80
0b8c718
c6499ef
3b2b4f7
56c10ba
cb0df78
7793de4
a42df99
b2971a9
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -14,7 +14,6 @@ const TutorialNavigationButton = React.forwardRef< | |
>(({ clickHandler, value, disabled = false, classes }, ref) => ( | ||
<a | ||
className={cx(anchorButtonRecipe({ variableWidth: true, smallGapX: true }), classes)} | ||
onClick={clickHandler} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. it's duplicated by There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, good catch. It doesn't make sense to have both. |
||
{...{ disabled }} | ||
{...fastClick(clickHandler)} | ||
ref={ref} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,260 @@ | ||
import { screen } from '@testing-library/dom' | ||
import userEvent from '@testing-library/user-event' | ||
import { act } from 'react' | ||
import createTestApp, { cleanupTestApp, cleanupTestEventHandlers } from '../../../test-helpers/createTestApp' | ||
|
||
// as per https://testing-library.com/docs/user-event/options/#advancetimers | ||
// we should avoid using { delay: null }, and use jest.advanceTimersByTime instead | ||
const user = userEvent.setup({ advanceTimers: jest.advanceTimersByTime }) | ||
|
||
/** Gets the last empty thought in the document. Mostly used after `user.keyboard('{Enter}')` to get the new thought. */ | ||
const lastEmptyThought = () => | ||
Array.from(document.querySelectorAll('[data-editable="true"]')) | ||
.filter(it => !it.textContent) | ||
.at(-1)! | ||
|
||
describe('Tutorial 1', async () => { | ||
beforeEach(() => createTestApp({ tutorial: true })) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maybe I'm missing something, but if the tests are all run sequentially on a single app instance, wouldn't this need to be There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I really have not much idea how to make it work with one right now it's run with one db/store/state/whatever, but with cleaning up event listeners that keep attaching I'm really not sure how to make it work with one There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks for your reply. I'd like to dig a little deeper into this. The tests are quite readable right now, but it's not clear what state they share between test blocks.
What do you mean by "empty-ish DOM"?
Are you saying that the current implementation recreates an isolated testing environment for each case? Because my understanding is that these tests depend on the current test step being preserved between tests.
The event listeners only keep attaching because you remove them and re-attach them each time.
Is it a sequential run or not? Or some hybrid? I think for clarity we need to stick with one or the other, or more carefully document the unique combination of shared state between test runs. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
we have everything in db, store etc., but dom is empty if we don't call the
Just runing a
but it does not work like this at all. describe.only('T_T', () => {
beforeAll(createTestApp)
it('we create "first" thought', async () => {
await user.keyboard('{Enter}')
await user.type(lastEmptyThought(), 'first')
await user.keyboard('{Enter}')
expect(exportContext(store.getState(), HOME_TOKEN)).toEqual(`<ul>
<li>__ROOT__
<ul>
<li>first</li>
<li></li>
</ul>
</li>
</ul>`)
expect(screen.getByText('first')).toBeInTheDocument()
})
it('we expect to see "first" thought', () => {
console.log(exportContext(store.getState(), HOME_TOKEN))
screen.debug(document.querySelector('.content')!) /* <-- prints this
<body
data-browser="safari"
data-color-mode="dark"
data-device="desktop"
data-drag-in-progress="false"
data-native="false"
data-platform="other"
/>
*/
expect(() => screen.getByText('first')).toThrow()
})
}) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. maybe we should "fix" this, but will it break other tests? i generally feel like this pr now achieves what we want it to, given the current testing environment There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks. I'd love to figure out why the DOM is empty. Without knowing the answer to that, it's hard to know what the right call is here. However, since all the tests pass and they are quite readable, I'm okay with merging the current implementation so that we have a good checkpoint in the commit history. We can investigate further in a separate issue. If you could address the small stylistic changes below, then I'd be happy to merge. Thanks! |
||
afterEach(cleanupTestEventHandlers) | ||
afterAll(cleanupTestApp) | ||
describe('step start', () => { | ||
it('we see the welcome text', () => { | ||
expect(screen.getByText('Welcome to your personal thoughtspace.')).toBeInTheDocument() | ||
}) | ||
|
||
it('we can proceed to first step by clicking Next', async () => { | ||
await user.click(screen.getByText('Next')) | ||
expect(screen.getByText('Hit the Enter key to create a new thought.')).toBeInTheDocument() | ||
}) | ||
}) | ||
|
||
describe('step first thought', () => { | ||
it('we learn how to create a thought', async () => { | ||
await user.keyboard('{Enter}') | ||
expect(screen.getByText('Now type something. Anything will do.')).toBeInTheDocument() | ||
}) | ||
|
||
it('we create our first thought', async () => { | ||
await user.type(lastEmptyThought(), 'first') | ||
await user.keyboard('{Enter}') | ||
await act(vi.runOnlyPendingTimersAsync) | ||
|
||
expect(screen.getByText('Well done!')).toBeInTheDocument() | ||
}) | ||
}) | ||
|
||
it('we create a second thought', async () => { | ||
expect(screen.getByText(/Try adding another thought/)).toBeInTheDocument() | ||
await user.type(lastEmptyThought(), 'second') | ||
await user.keyboard('{Enter}') | ||
await act(vi.runOnlyPendingTimersAsync) | ||
|
||
expect(screen.getByText('Now type some text for the new thought.')).toBeInTheDocument() | ||
}) | ||
|
||
it('we create a third thought', async () => { | ||
await user.type(lastEmptyThought(), 'third') | ||
await user.keyboard('{Enter}') | ||
await act(vi.runOnlyPendingTimersAsync) | ||
|
||
expect(screen.getByText(/Hit the Delete key to delete the current blank thought/)).toBeInTheDocument() | ||
}) | ||
|
||
it('we learn to create nested thoughts', async () => { | ||
/* thoughts: | ||
- first | ||
- second | ||
- third | ||
- | ||
*/ | ||
await user.keyboard('{Backspace}') | ||
await user.keyboard('{Control>}{Enter}{/Control}') | ||
await act(vi.runOnlyPendingTimersAsync) | ||
|
||
await user.type(lastEmptyThought(), 'child of third') | ||
await act(vi.runOnlyPendingTimersAsync) | ||
|
||
expect(screen.getByText(/As you can see, the new thought "child of third" is nested/)).toBeInTheDocument() | ||
}) | ||
|
||
describe('step autoexpand', async () => { | ||
it('we learn about automatic thought hiding', async () => { | ||
await user.click(screen.getByText('Next')) | ||
expect(screen.getByText(/thoughts are automatically hidden when you click away/)).toBeInTheDocument() | ||
}) | ||
|
||
it('we hide nested thoughts by clicking away', async () => { | ||
/* thoughts: | ||
- first | ||
- second | ||
- third | ||
- child of third | ||
*/ | ||
await user.click(screen.getByText('second')) | ||
expect(screen.getByText(/Notice that "child of third" is hidden now/)).toBeInTheDocument() | ||
}) | ||
|
||
it('we reveal hidden thoughts by clicking parent', async () => { | ||
await user.click(screen.getAllByText('third').at(-1)!) | ||
expect(screen.getByText('child of third')).toBeInTheDocument() | ||
}) | ||
}) | ||
|
||
describe('step success', async () => { | ||
it('we complete the first tutorial', async () => { | ||
expect(screen.getByText(/Lovely\. You have completed the tutorial/)).toBeInTheDocument() | ||
}) | ||
|
||
it('we can exit tutorial mode', async () => { | ||
await user.click(screen.getByText('Play on my own')) | ||
await act(vi.runOnlyPendingTimersAsync) | ||
expect(() => screen.getByTestId('tutorial-step')).toThrow('Unable to find an element') | ||
}) | ||
|
||
it('we can continue to advanced tutorial', async () => { | ||
expect(screen.getByText('Learn more')).toBeInTheDocument() | ||
}) | ||
Comment on lines
+114
to
+116
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There's a contradiction here: If the user just exited the tutorial, then how could the "Learn more" button be visible? This seems to rely on the tutorial inadvertently being re-launched on
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. since I can do it, given the circumstances, I left it as is (test of a test? xd) |
||
}) | ||
}) | ||
|
||
describe('Tutorial 2', async () => { | ||
beforeEach(() => createTestApp({ tutorial: true })) | ||
afterEach(cleanupTestEventHandlers) | ||
afterAll(cleanupTestApp) | ||
it('we learn about context indicators', async () => { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's more accurate to say "contexts" or "context view" than "context indicators". Let's update this to be more consistent with em terminology. Likewise below. |
||
await user.click(screen.getByText('Learn more')) | ||
|
||
expect(screen.getByText(/shows a small number to the right of the thought, for example/)).toBeInTheDocument() | ||
}) | ||
|
||
it('we choose a project type, one of three options', async () => { | ||
await user.click(screen.getByText('Next')) | ||
expect(screen.getByText(/For this tutorial, choose what kind of content you want to create/)).toBeInTheDocument() | ||
expect(screen.getByText('To-Do List')).toBeInTheDocument() | ||
expect(screen.getByText('Journal Theme')).toBeInTheDocument() | ||
expect(screen.getByText('Book/Podcast Notes')).toBeInTheDocument() | ||
}) | ||
|
||
it('we start creating a to-do list', async () => { | ||
await user.click(screen.getAllByText('To-Do List').at(-1)!) | ||
expect(screen.getByText(/Excellent choice\. Now create a new thought with the text “Home”/)).toBeInTheDocument() | ||
}) | ||
|
||
describe('step context 1 - create a "Home" to-do list', () => { | ||
it('we create a Home thought', async () => { | ||
await user.keyboard('{Enter}') | ||
await user.type(lastEmptyThought(), 'Home') | ||
await act(vi.runOnlyPendingTimersAsync) | ||
expect(screen.getByText(/Add a thought with the text "To Do"/)).toBeInTheDocument() | ||
}) | ||
|
||
it('we add a Home->To Do subthought', async () => { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please use Likewise below. |
||
await user.keyboard('{Control>}{Enter}{/Control}') | ||
await user.type(lastEmptyThought(), 'To Do') | ||
await act(vi.runOnlyPendingTimersAsync) | ||
expect(screen.getByText(/Now add a thought to “To Do”/)).toBeInTheDocument() | ||
}) | ||
|
||
it('we add a Home->To Do->or to not subthought sub-subthought', async () => { | ||
await user.keyboard('{Control>}{Enter}{/Control}') | ||
await user.type(lastEmptyThought(), 'or to not') | ||
await act(vi.runOnlyPendingTimersAsync) | ||
|
||
/* thoughts: | ||
- Home | ||
- To Do | ||
- or to not | ||
*/ | ||
expect(screen.getByText(/Nice work!/)).toBeInTheDocument() | ||
}) | ||
}) | ||
|
||
describe('step context 2 - create a "Work" to-do list', () => { | ||
it('we prepare to create another list', async () => { | ||
await user.click(screen.getByText('Next')) | ||
expect(screen.getByText(/Now we are going to create a different "To Do" list./)).toBeInTheDocument() | ||
}) | ||
|
||
it('we create a Work thought', async () => { | ||
// we created a new thought on 3rd level, clicking "Home" gets us to root | ||
await user.click(screen.getByText('Home')) | ||
await user.keyboard('{Enter}') | ||
await user.type(lastEmptyThought(), 'Work') | ||
await act(vi.runOnlyPendingTimersAsync) | ||
|
||
/* thoughts: | ||
- Home | ||
- To Do | ||
- or to not | ||
- Work | ||
*/ | ||
expect(screen.getByText(/Now add a thought with the text "To Do"/)).toBeInTheDocument() | ||
}) | ||
|
||
it('we add another Work->To Do thought', async () => { | ||
await user.keyboard('{Control>}{Enter}{/Control}') | ||
await user.type(lastEmptyThought(), 'To Do') | ||
expect(screen.getByText('Imagine a new work task. Add it to this “To Do” list.')) | ||
}) | ||
|
||
it('we see context indicators', async () => { | ||
expect(screen.getAllByRole('superscript')[0]).toHaveTextContent('2') | ||
expect(screen.getByText(/This means that “To Do” appears in two places/)).toBeInTheDocument() | ||
}) | ||
|
||
it('we add a Work->To Do->Text boss', async () => { | ||
await user.keyboard('{Control>}{Enter}{/Control}') | ||
await user.type(lastEmptyThought(), 'Text boss') | ||
await act(vi.runOnlyPendingTimersAsync) | ||
/* thoughts: | ||
- Home | ||
- To Do | ||
- or to not | ||
- Work | ||
- To Do | ||
- Text boss | ||
*/ | ||
expect(screen.getByText('Next')).toBeInTheDocument() | ||
}) | ||
}) | ||
|
||
describe('step context view open', () => { | ||
it('we learn about multiple contexts', async () => { | ||
await user.click(screen.getByText('Next')) | ||
await act(vi.runOnlyPendingTimersAsync) | ||
expect(screen.getByText(/First select "To Do"./)).toBeInTheDocument() | ||
}) | ||
|
||
it('we select a thought with multiple contexts', async () => { | ||
await user.click(screen.getAllByText('To Do').at(-1)!) | ||
await act(vi.runOnlyPendingTimersAsync) | ||
expect(screen.getByText("Hit Alt + Shift + S to view the current thought's contexts.")).toBeInTheDocument() | ||
}) | ||
|
||
it('we view contexts of a thought', async () => { | ||
await user.keyboard('{Alt>}{Shift>}S{/Shift}{/Alt}') | ||
await act(vi.runOnlyPendingTimersAsync) | ||
expect( | ||
screen.getByText(/We now see all of the contexts in which "To Do" appears, namely "Home" and "Work"./), | ||
).toBeInTheDocument() | ||
}) | ||
}) | ||
|
||
it('we see real-world context examples', async () => { | ||
await user.click(screen.getByText('Next')) | ||
expect(screen.getByText(/Here are some real-world examples of using contexts in/)).toBeInTheDocument() | ||
}) | ||
|
||
describe('step success', () => { | ||
it('we complete the advanced tutorial', async () => { | ||
await user.click(screen.getByText('Next')) | ||
expect(screen.getByText(/Congratulations! You have completed Part II of the tutorial./)).toBeInTheDocument() | ||
}) | ||
|
||
it('we exit the tutorial', async () => { | ||
user.click(screen.getByText('Finish')) | ||
await act(vi.runOnlyPendingTimersAsync) | ||
expect(() => screen.getByTestId('tutorial-step')).toThrow('Unable to find an element') | ||
}) | ||
}) | ||
}) |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -13,10 +13,9 @@ import storage from '../util/storage' | |
let cleanup: Await<ReturnType<typeof initialize>>['cleanup'] | ||
|
||
/** Set up testing and mock document and window functions. */ | ||
const createTestApp = async () => { | ||
const createTestApp = async ({ tutorial }: { tutorial?: boolean } = {}) => { | ||
trevinhofmann marked this conversation as resolved.
Show resolved
Hide resolved
|
||
await act(async () => { | ||
vi.useFakeTimers({ loopLimit: 100000 }) | ||
|
||
// calls initEvents, which must be manually cleaned up | ||
const init = await initialize() | ||
cleanup = init.cleanup | ||
|
@@ -33,8 +32,8 @@ const createTestApp = async () => { | |
) | ||
|
||
store.dispatch([ | ||
// skip tutorial | ||
{ type: 'tutorial', value: false }, | ||
// there are cases where we want to show tutorial on test runs, whilst mostly we don't | ||
{ type: 'tutorial', value: !!tutorial }, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Instead of using There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. FYI This is a small concession to allow point-free style in See: #2702 (comment) |
||
|
||
// close welcome modal | ||
{ type: 'closeModal' }, | ||
|
@@ -80,4 +79,13 @@ export const refreshTestApp = async () => { | |
await act(vi.runOnlyPendingTimersAsync) | ||
} | ||
|
||
/** Clear existing event listeners(e.g. keyboard, gestures), but without clearing the app. */ | ||
export const cleanupTestEventHandlers = async () => { | ||
raineorshine marked this conversation as resolved.
Show resolved
Hide resolved
|
||
await act(async () => { | ||
if (cleanup) { | ||
cleanup() | ||
} | ||
}) | ||
} | ||
|
||
export default createTestApp |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it was a bug I've found while testing