React Interview test submission - #1

On Mar 19, 2020, at 11:48 AM, John Moose ***@***.***> wrote:  @jmooserific requested changes on this pull request. Hi, @elliotgonzalez123. Thanks for working on this! The following is how I would reply if you worked at LendKey. Your pull request definitely meets the Acceptance Criteria in the ticket. Good job! 👍 I can tell you have some TypeScript experience. Unfortunately, this doesn't meet our code quality standards because the test coverage isn't actually at 100%. (I suspect that you were running npm test in "watch" mode, so it wasn't calculating test coverage for the whole app, but just for your changes.) Try running it like this instead: CI=true nom test. If you merge in my most recent change from the original repo, I made this a little easier by ignoring serviceWorker.ts when calculating test coverage. You also left a couple of debugging console.log()s behind. We usually remove things like that before deploying our code. Feel free to delete my sample code instead of commenting it out. Pretend like you own this codebase and will be maintaining it! 💪 You are welcome to make changes, just like in the real world. If anything I said doesn't make sense, please feel free to ask questions here. Thanks! In src/components/rocketTable/RocketTable.test.tsx: > @@ -27,7 +27,7 @@ describe("RocketTable component", () => { const table = await findByTestId("rockets"); const { getAllByRole } = within(table); expect(getAllByRole("columnheader")).toHaveLength(3); - expect(getAllByRole("row")).toHaveLength(5); + expect(getAllByRole("row")).toHaveLength(31); This is still making a real API call. (As you mentioned above, this makes your test slow and fragile.) Please update this test so that the API call is mocked, just like you did in src/api/rockets/index.test.ts. I might add two more "integration-y" tests here. "With no results", where your axios mock returns an empty array of rockets, and "With errors", where your axios mock returns an error. You could then get rid of the two super fake tests below. There's an example of mocking a network error here. In src/api/rockets/index.ts: > }; // Call the API endpoint and return the response body export const getRocketsList = async () => { try { - const data: JsonResponse = JSON.parse(await getRockets()); + //I do not believe parsing an axios response is necessary so I removed it. + //Had to troubleshoot for a few minutes why data was not passing through, the culprit was the parse function. Exactly right. Axios parses the JSON for you. In src/api/rockets/index.ts: > // Simulate calling the API endpoint https://launchlibrary.net/1.4/rocket?mode=list export const getRockets = async () => { + //leaving this in for dramatic effect! await new Promise(resolve => setTimeout(resolve, 500)); // Wait 0.5s for dramatic effect I'd remove this, because the real API call adds enough drama. 😄 In src/api/rockets/index.ts: > }; // Call the API endpoint and return the response body export const getRocketsList = async () => { try { - const data: JsonResponse = JSON.parse(await getRockets()); + //I do not believe parsing an axios response is necessary so I removed it. + //Had to troubleshoot for a few minutes why data was not passing through, the culprit was the parse function. + const data: JsonResponse = await getRockets(); What happens if the API service returns an error? (Like when the site is down.) — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or unsubscribe.