foundersandcoders / express-and-testing-workshop Goto Github PK

We've just been introduced to Express via a walkthrough - so it would be nice if the file structure and app structure reflected what we had just been over to avoid a mind-crushing headache for the first 15 minutes of the workshop.

The 'hints' felt more like riddles to begin with - if the focus of this workshop is testing, maybe the server should be fully built so we could focus on the testing, which is still enough of a challenge for most people.

Also...

• Why does this app use a port variable, when we've just been encouraged to use app.set('port')?
• Why is path required in server.js? Did I miss something?

what is the purpose of these steps?

• npm start to begin the project.

• In another terminal pane run npm test.

This would make more sense if students needed to look at the browser for something, or needed to use the tests for something. But FAC11 weren't instructed to do this before looking at server.js

I would put the two setup files in database in a top level scripts or utils folder.

here and in most tests, the test is duplicated because there is a .expect(200) above

There are no real instructions for the actual tasks that students are supposed to complete in this workshop

• Add instructions to readme

How should students know they are supposed to reach this solution from:

Fill this with many many tests YAY!!

Some verbal responses from FAC11

Where do I start?

So what am I supposed to test?

This made it necessary for @Akin909 and @PiotrBerebecki to float around the room, providing a lot of verbal instruction and explanation.

Most students were therefore looking at the solutions branch to figure out what to write. This doesn't really teach them to reason about what tests they would write for themselves. Using the solution branch to figure it out led to questions like:

Why am I testing whether an array is an array?

What's a good thing to test?

With the POST request, what are you wanting us to test?

You might need to give some pointers on setting up the db + starting server etc in tests

Again here is a rough example

https://github.com/foundersandcoders/open-tourism-platform

Linux Users aren't able to to do this workshop, unless in the dbConnection.js file, you add a database user and password(You can add the details of the user you created on week6). Please see below examples:

`const localDB = {

host: 'localhost',
port: 5432,
database: 'fac-express',
user: 'XXXXXXXX',
password: 'XXXXXXX'
};`

The readme suggests that this is TDD, but writing tests after the code already exists is not "test-driven" development.

It's impossible to go through the red, green, refactor cycle when you already have the code.

Use TDD to develop a backend server/API

Express routes can be tested using TDD

This repository provides you with a skeleton express server your task is to create tests for your express routes using TDD.

Like the "learning outcomes" heading, have a "prerequisites" heading.

This was stated verbally by @Akin909 but isn't evident from the repo

@Akin909, you did a postman demo in person.

Have you used postman before? Well, you're going to need this a lot during this workshop.

In order to make this replicable in other campuses, we need some kind of giffy / video of your demo either: inside the readme, for students to watch, or inside a mentor-notes folder for mentors to use to prepare

this is not ideal, we don't want to have to change the contents for each cohort

I would definitely split up that tasks list a bit. There's suggestions, installs and notes stuck in there.
Maybe something like:

Prerequisites

...

Setup

...

Tasks

...

Notes

...

Resources

...

Same as the other workshops

@Akin909 spent 10 mins at the start of the workshop walking through supertest.

Ideally, the verbal explanations given to FAC11 would be explicitly written in this repo's readme (instead of just linking to the docs in the tasks), so this workshop can stand alone.

some ubuntu users not able to connect to the db after following the instructions. (The error was about not having permission to access the db)

one solution was to [add a new superuser with psql and] add user and password fields to the pg options here

This should be added to the README or the options added to the db connection file to be filled in.

"facsters" caused a little bit of (needless?) confusion

The workshop says:

The structure of your tests should be as below. Note that supertest is assigned to request as this is a convention.

const test = require('tape')

test('What your tests is testing', (t) => {
    request(app)
      .get('/facsters')
      .expect(200)
      .end(function(err, res) {
        /* INSERT TAPE TESTS HERE
        Don't forget to end your test */
      })
    })

but the premade files say:

const tape = require('tape');
const supertest = require('supertest');

This caused confusion for nearly everyone, who thought we needed to import the request module.

At the moment the solution branch is different to the master branch. The queries file has been moved to a seperate folder and is also slightly different, below is 1 function which changes

Master:
https://github.com/foundersandcoders/express-and-testing-workshop/blob/master/server/routes/queries.js#L17-L20
Solution
https://github.com/foundersandcoders/express-and-testing-workshop/blob/solution-branch/server/queries/index.js#L18-L21

This isn't a massive issue I guess, but it causes a lot of confusion. The solution tests in the solution branch don't work on the master branch - that doesn't make much sense and if students have been working on the master branch (as they should) and then look at the tests on the solution branch they will (and have) got quite confused

@Akin909 ended up casting the solution so that students could move forward

It's not overly clear what students are being asked to do. Some of them seemed surprised that they were only being asked to write these 2 lines. Others didn't understand exactly what was happening here.

It would help to

• have clear instructions in the readme, rather than the file itself
• walk students through the existing codebase before setting them a task

Getting students to write app.use(bodyParser.json());

In the readme:

inside server.js require in any necessary middleware

How should students know what middleware is necessary at this point?

Then, in the server.js file itself:

We need middleware to convert request and response payloads to an from JSON

Getting students to write app.use('/v1/api/', routes);

Then, in the server.js file itself:

We need to setup a route/collection of routes

and this is sort of in the introduction section of the readme

// In Server.js
const app = express()
app.use('/api/v1/', routes)

The fact that students didn't know what to do was partly caused by the confusion around step 13 (modularisation) in day 1's code along.

After casting the solution, the verbal feedback from students was:

I don't really know what this line is doing

Can you do this on one line?

Why are you pointing to a whole folder instead of a file?

(i.e. const routes = require('./routes');)

I believe that we should remove all references to '/api/v1/' in this task, as it just make the workshop more confusing. This was an issue for some of FAC 13.

The readme should have more details on how to use supertest and what is does. The official documentation didnt guide students well through the exercise as it doesnt really go over integration with tap.

One of the biggest issues was people testing the response code twice - in supertest and tape. Possibly a more detailed example or more information is needed?

I think the convention more like:

-- server
  |-- models
     |-- ..
  |-- controllers
     |-- some_controller.js
     |-- some_other_controller.js
  |-- router.js
  |-- app.s
-- tests

But I could be wrong here I have not looked at that many express projects..

https://github.com/foundersandcoders/express-and-testing-workshop/blob/master/server/routes/index.js#L12-L24

Edit: FACK1 express and testing workshop

• Edited exercise instructions:

## Exercise

### Endpoints

* `/facsters`
* `/facsters/:name` e.g. `facsters/amelie`
* `/facster/new` - This is a post request expecting an object
* `/facsters/:name/superpower`
* `/facsters/:name/hobby`

### Write tests for the following:

1. All routes including `/facsters` should return the expected result: 200 status code and content type JSON
2. First user should be 'Abdullah'
3. Should be able to get a facster by their name
4. Should be able to add a new facster
5. Should find a facster's hobbies
6. Should return a facster's superpower

https://github.com/jema28/express-and-testing-workshop

• Moved postman from requirements to notes
• Removed the majority of references to supertest

We just delivered a fork of this workshop: https://github.com/jema28/express-and-testing-workshop.
I think we need to clarify the aim of the workshop: is it to look at express code and decide which tests to write (open exercise / limited guidance), a testing workshop that focuses purely on learning tape and supertest, or a hybrid of the two?

@shahenazmonia and I decided to add an exercise section with specific routes to test #33. In hindsight maybe we needed to find a balance between giving too much away vs leaving it completely open / up to the cohort.

Any thoughts?

☝️ this is never used

Looks like pg now supports Promises, as well as Callbacks:

https://node-postgres.com/features/queries

Can we remove pg-promise? Students in FACN7 found this syntax confusing anyway:

const pgp = require('pg-promise')();