Skip to content
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

Testing: Add additional Puppeteer tests to check Sprint creation & updating #218

Open
8 tasks
jayeclark opened this issue Apr 23, 2022 · 15 comments
Open
8 tasks
Assignees
Labels
good first issue Good for newcomers and newer coders help wanted This issue is still active and help is wanted on it! Puppeteer Involves using the Puppeteer testing library test Involves adding or improving test coverage

Comments

@jayeclark
Copy link
Collaborator

Description

We recently implemented a GitHub Actions CI flow to run automated unit & integration tests on newly opened pull requests. The tests are very limited right now - really just proof of concept that unit and integration tests will work.

Are you looking to practice Puppeteer or your test-writing skills? Or are you just a wizard at integration testing who likes helping out on open source efforts? We would love it if you could help us establish some best practices in testing by expanding the initial set of integration tests that are run. We'd like to focus on Sprints & the Arena.

Background

The 'Arena' section of the app is a place where learners can create weekly sprints to help them practice good habits related to their software development career. The main screen shows your upcoming, current, and completed sprints in chronological order, starting with the most recent one.

Clicking on any one of them will bring you to the individual Sprint view. In this view you can record & view your planning notes, mark daily achievement goals as completed (use Time Travel to submit achievements for previous days if you forgot to record them), and view your position in the leaderboard of the team participating in your sprint.

To create a new sprint, click on "Start a Sprint" in the left-hand navigation panel. You'll have the option to select a Sprint Template from existing predefined templates, Competitors (friends you want to join you in the sprint), and a Start Date. Once you click the 'create' button, you'll be brought back to the main page showing all your sprints.

To create a template for a sprint, click on "Sprint Template". Enter a name for your template, drag & drop achievements to the time of day that you aim to complete them, and click the "create" button to save your template.

Tests

  • A user can create a sprint template
  • A user can create a sprint
  • Clicking on a sprint displays details about that sprint
  • A user can save planning notes in the Plan tab of Sprint details
  • A user can report completing an achievement if the sprint is currently active
  • A user cannot report achievements for inactive sprints
  • A user can report an achievement on an earlier or later day by using time travel
  • When a user reports an achievement, their progress is shown in the 'stats' and their position is shown on the leaderboard
@jayeclark jayeclark added help wanted This issue is still active and help is wanted on it! good first issue Good for newcomers and newer coders test Involves adding or improving test coverage Puppeteer Involves using the Puppeteer testing library labels Apr 23, 2022
@jayeclark jayeclark added this to the Testing, Testing milestone Apr 23, 2022
@houssamboudiar
Copy link
Contributor

Interested !

@jayeclark
Copy link
Collaborator Author

@houssamboudiar Assigned!

@houssamboudiar
Copy link
Contributor

@houssamboudiar Assigned!

Just started testing "Sprint templates creation" => CreateSprintTemplate.tsx and my tests don't seem to be running cause of that :
Warning: Failed prop type: Invalid prop error of type string supplied to ForwardRef(TextField2), expected boolean.
Screenshot from 2022-08-31 22-57-58

Any insights ?

@jeremydthomas
Copy link
Collaborator

I'm out and about but I'll have a look see when I get a chance.

@jeremydthomas
Copy link
Collaborator

oof, I know nothing about puppeteer, this happens when you try to run a puppeteer test on that file?

@houssamboudiar
Copy link
Contributor

houssamboudiar commented Sep 1, 2022

oof, I know nothing about puppeteer, this happens when you try to run a puppeteer test on that file?

That error occurs even when you try to create sprint template ( browser render ), So when i try to render (render from '@testing-library/react') this component in order to test it ... my tests fails cause of it (i think it has nothing to do with puppeteer)

RUNS  src/tests/Sprint.spec.tsx
node:internal/process/promises:288
            triggerUncaughtException(err, true /* fromPromise */);
            ^

[UnhandledPromiseRejection: This error originated either by throwing inside of an async function without a catch block, or by rejecting a promise which was not handled with .catch(). The promise rejected with the reason "API pareto does not exist".] {
  code: 'ERR_UNHANDLED_REJECTION'
}

Node.js v18.7.0
/home/houssamboudiar/paretOS/node_modules/.pnpm/execa@5.1.1/node_modules/execa/lib/error.js:60
                error = new Error(message);
                        ^

@jayeclark
Copy link
Collaborator Author

Can you put up a draft PR showing your code so far so I can take a look & try to help?

I’ll also see if I can find a quick ‘here’s what puppeteer does & how to write its tests’ @jeremydthomas . If you’re familiar with or have heard of Cypress, Selenium, Ansible or Playwright, it’s in that family of testing architectures. Instead of jest unit tests that look at isolated components, Puppeteer basically pretends to be a person going to the complete site (using a headless browser - pretty neat!), taking various actions a user might, and then with each action it checks that the whole site and all the components are working together as expected. Takes a little effort to get the hang of but is an important CI step in making sure the complete app works as intended. (Sorry if this overview is too basic, I figured I’d start from first principles 😁)

@houssamboudiar
Copy link
Contributor

houssamboudiar commented Sep 1, 2022

Can you put up a draft PR showing your code so far so I can take a look & try to help?

I’ll also see if I can find a quick ‘here’s what puppeteer does & how to write its tests’ @jeremydthomas . If you’re familiar with or have heard of Cypress, Selenium, Ansible or Playwright, it’s in that family of testing architectures. Instead of jest unit tests that look at isolated components, Puppeteer basically pretends to be a person going to the complete site (using a headless browser - pretty neat!), taking various actions a user might, and then with each action it checks that the whole site and all the components are working together as expected. Takes a little effort to get the hang of but is an important CI step in making sure the complete app works as intended. (Sorry if this overview is too basic, I figured I’d start from first principles grin)

Just to add to your constructive comment, So we basically have these type of tests :

  • Unit testing (Jest).
  • End to end testing - E2E (Cypress, Puppeteer, Selenium).
  • Integration tests (You basically test more than one component at the same time).

So Jay while im working on " A user can create a sprint template"
I wanted just to follow your code "Board.spec.tsx" and do some unit tests on CreateSprintTemplate.tsx :
Sprint.spec.tsx

describe("SPRINT", () => {
    const users = testUsers();
      describe("Leaderboard Table with 3 users per page and a current user", () => {
          beforeEach(() => {
          render(
              <CreateSprintTemplate
                user={ users[1] }
                history={[]}
              />
          );
          });
          it("displays a sprint H1", () => {
          expect(screen.getByTitle("CreateTemplate")).toBeDefined();
          });
      });
    }
);

But when that async api call comes :
CreateSprintTemplate.tsx

  useEffect(() => {
    getConfiguration();
  }, []);

  // pulls the /templates api and sets the existing templates
  async function getConfiguration() {
    let options = await RestAPI.get("pareto", "/templates", {});
    setExistingTemplates(options.map((option: { title: any }) => option.title));
  }

That error occurs :

[UnhandledPromiseRejection: This error originated either by throwing inside of an async function without a catch block, or by rejecting a promise which was not handled with .catch(). The promise rejected with the reason "API pareto does not exist".] {
code: 'ERR_UNHANDLED_REJECTION'
}

I think using Puppeteer would be better for testing that functionality, I'll be working on it ...

@jayeclark
Copy link
Collaborator Author

Ahhh I understand the context better now. Yeah, I think calling the API via a unit test will throw a big error like you’re seeing, you’d basically have to mock it so that when the api call happens, the test framework sends back a dummy response.

I think you’re on the right track - easier to test with puppeteer and not deal with mocks. (I’m struggling with them at work right now haha.)

@houssamboudiar
Copy link
Contributor

Ahhh I understand the context better now. Yeah, I think calling the API via a unit test will throw a big error like you’re seeing, you’d basically have to mock it so that when the api call happens, the test framework sends back a dummy response.

I think you’re on the right track - easier to test with puppeteer and not deal with mocks. (I’m struggling with them at work right now haha.)

Hhhhh yes, it would have been easier if the component was more pure so just have to pass it via props then test ..., I'm on puppeteer docs right now ! i think it's not added to jest configuration yet ?

@jayeclark
Copy link
Collaborator Author

Puppeteer runs as a separate process from jest. I’m not at my PC so I can’t recall if I did it for this repo, but usually I add it as a script to the package.json. I think this was one of my first times implementing puppeteer from scratch so I may have failed to do that! (It runs automatically during the CI process when a pull request is raised, but you can trigger it locally using the same command.)

I think you’ve uncovered an important issue/potential opportunity for improvement in how some of the components are designed — we should have probably abstracted out any unavoidable calls to an API to a separate file. That would make it a lot more straightforward to mock the results of those API calls when unit testing! I will probably add that as an issue this weekend.

@jeremydthomas
Copy link
Collaborator

jeremydthomas commented Sep 1, 2022

@jayeclark I'm familiar with cypress, I've modified some unit tests and I've used it to test things locally. Ill have to start looking at docs and working with puppeteer as well.

@jeremydthomas
Copy link
Collaborator

I thought we could close this @jayeclark , but then I wasnt sure.

@jayeclark
Copy link
Collaborator Author

Hmmm…. Yeah @jeremydthomas I think we should close the issue and reopen it with a new description. We don’t have any integration tests for sprint creation (no unit tests either) but most of the instructions/discussion are no longer relevant since we’re using vitest for the integ tests now instead of Puppeteer.

@jeremydthomas
Copy link
Collaborator

@jayeclark ,if you put up an issue with what you want, I'll attempt making the tests.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers and newer coders help wanted This issue is still active and help is wanted on it! Puppeteer Involves using the Puppeteer testing library test Involves adding or improving test coverage
Projects
None yet
Development

No branches or pull requests

3 participants