Testing setup #142

promaty · 2020-04-22T02:26:04Z

Added continuous testing for every commit. This way we can see if all tests are passing while reviewing a PR. Tests are continuously running and creating an image with a :rolling tag. When we finally merge into development/master+tag that rolling image is tagged with :latest/:v* and deployed to k8s.

Currently for testing I just set up a server+postgres with docker-compose and check all localhost api endpoints for correct response. We can add more tests later on.

Tests can also be run locally using docker-compose exec court_server npm run test:server.

This also temporarily fixes the session bug that was blocking front end development.

facuspagnuolo

Looking really good!

facuspagnuolo · 2020-04-22T12:51:19Z

docker-compose.yml

@@ -19,6 +19,7 @@ services:
      - "9091:9091" # for debugging metrics
    volumes:
      - ./packages/server/src:/court-backend/packages/server/src
+      - ./packages/server/test:/court-backend/packages/server/test


What's the rationale for adding the test dir here?

You can update and run tests while developing without rebuilding the container, much faster this way

facuspagnuolo · 2020-04-22T12:52:29Z

packages/server/package.json

    "morgan": "^1.9.1",
+    "objection": "^2.1.3",


testing dependencies should be devDependencies actually

facuspagnuolo · 2020-04-22T12:53:15Z

packages/server/src/helpers/session-middleware.js

@@ -25,13 +25,12 @@ export default () => {
  })

  return session({
-    store,
-    key: 'aragonCourtSessionID',
+    // store,   // I disabled this for now because it doesn't work and I will be migrating to Objection in the next PR


Let's merge this PR once we have the whole refactor then

But front end guys need something functional right now so we can have it working like this (as it was working in my first implementation already)

Can't we simply store a session for the users then? It would be simply doing this I think

Oh if it's so simple I will fix it.

Hmm in order to make this work your model forces me to use users table but I don't want to touch it right now while we are still actively developing. I would prefer to merge this in memory solution now while I figure out the storage stuff which will still take a day or so.

added your fix, thanks!

facuspagnuolo · 2020-04-22T12:53:56Z

packages/server/src/helpers/session-middleware.js

    resave: false,                                      // Don't force sessions to be saved back to the store even if they didn't change
    saveUninitialized: false,                           // Don't force uninitialized session to be saved to the store
    secret: process.env.SESSION_SECRET,                 // Secret used to generate session IDs
+    name: 'aragonCourtSessionID',                       // Cookie name to be used


facuspagnuolo · 2020-04-22T12:55:46Z

packages/server/src/routes/index.js

@@ -10,6 +10,9 @@ export default app => {
  // check user details
  app.get(    '/users/:address',                      asyncMiddleware(users.details))

+  // verify user email using provided token (needs to come before sessions to avoid session authentication)
+  app.post(   '/users/:address/email[:]verify',       asyncMiddleware(users.email.verify))


Let's have something similar to what we did with the admin authenticated routes. I prefer being explicit than depend on how we order routes, we can mess it up easily

Hmm this requires a separate controller, which also requires model, which will depend on storage, I prefer to do this in the next PR

facuspagnuolo · 2020-04-22T12:56:35Z

packages/server/test/integration/server-endpoints.js

+// agent is used to persist authentication cookie across multiple tests
+const agent = chai.request.agent(`http://localhost:${serverPort}`);
+
+describe("Server Endpoints", () => {


I know #134 hasn't been implemented yet, but let's try to use single quotes for all strings and no semicolons 🙏

facuspagnuolo · 2020-04-22T12:59:05Z

packages/server/test/integration/server-endpoints.js

+chai.use(chaiHttp);
+
+// agent is used to persist authentication cookie across multiple tests
+const agent = chai.request.agent(`http://localhost:${serverPort}`);


The proper way to do this with mocha is to use a before statement and an after statement to close it

Hmm before statement doesn't work, agent variable needs to be reachable inside other it scopes. I can only use after to close it.

facuspagnuolo · 2020-04-22T13:00:39Z

packages/server/package.json

@@ -10,7 +10,8 @@
  "scripts": {
    "build": "babel ./src --out-dir ./build --source-maps --copy-files",
    "start": "npm run build && ./scripts/db-setup.sh && node ./build",
-    "start:dev": "./scripts/db-setup.sh && nodemon --ignore ./build --exec babel-node ./src/index.js"
+    "start:dev": "./scripts/db-setup.sh && nodemon --ignore ./build --exec babel-node ./src/index.js",
+    "test": "npx mocha test --recursive"


👏👏👏👏

facuspagnuolo · 2020-04-22T13:01:52Z

.github/workflows/testnet.yml

+        export SERVER_IMAGE=$REPO:${GITHUB_SHA}
+        docker-compose up -d
+        sleep 10
+        docker-compose exec -T court_server npm run test:server


Isn't there a way to have a shared testing workflow that both deployments could depend on instead of having it repeated for both?

Yeah that's a good idea, I will add a shared shell script.

facuspagnuolo · 2020-04-22T13:10:11Z

.github/workflows/testnet.yml

@@ -2,7 +2,10 @@ name: Testnet CI/CD
 on:
  push:
    branches:
-      - development
+    # Executes on any non master commit, but release and deploy steps only run on development
+    # This is useful to see if tests are passing during PR review


For sure :)

* set up continuous testing for server endpoints * added ci/cd branch wildcard * fixed ci/cd files * fixed conditional ref * added missing checkout and better docker caching * run all test actions in a single script * switch places docker --cahce-from for rolling updates first * disabled tty for docker-compose exec * added missing checkout for master ci/cd * added sleep 10 before running test * use single quotes and no semicolons * small fixes * moved some ci/cd logic into shared shell scripts * made scripts executable * added .git to dockerignore for better caching * cherry pick session storage fix * re-enabled session storage Co-authored-by: Facu Spagnuolo <[email protected]>

promaty added 10 commits April 22, 2020 03:14

set up continuous testing for server endpoints

afdf30a

added ci/cd branch wildcard

c134701

fixed ci/cd files

5c40e8c

fixed conditional ref

7167dd4

added missing checkout and better docker caching

f3f83b8

run all test actions in a single script

7b706f2

switch places docker --cahce-from for rolling updates first

f5a12b0

disabled tty for docker-compose exec

1f4ff25

added missing checkout for master ci/cd

57d64b5

added sleep 10 before running test

87a13d6

promaty requested review from sohkai and facuspagnuolo April 22, 2020 02:26

promaty assigned facuspagnuolo Apr 22, 2020

facuspagnuolo reviewed Apr 22, 2020

View reviewed changes

facuspagnuolo assigned promaty and unassigned facuspagnuolo Apr 22, 2020

promaty added 5 commits April 22, 2020 20:21

use single quotes and no semicolons

f2f675a

small fixes

bcc295d

moved some ci/cd logic into shared shell scripts

9b2569a

made scripts executable

396a4a9

added .git to dockerignore for better caching

1a6106f

promaty requested a review from facuspagnuolo April 22, 2020 19:52

facuspagnuolo and others added 2 commits April 22, 2020 22:20

cherry pick session storage fix

b488fa2

re-enabled session storage

68062d3

facuspagnuolo merged commit 2789682 into development Apr 22, 2020

facuspagnuolo deleted the testing_setup branch April 22, 2020 20:38

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Testing setup #142

Testing setup #142

promaty commented Apr 22, 2020 •

edited

Loading

facuspagnuolo left a comment

facuspagnuolo Apr 22, 2020

promaty Apr 22, 2020

facuspagnuolo Apr 22, 2020

facuspagnuolo Apr 22, 2020

promaty Apr 22, 2020

facuspagnuolo Apr 22, 2020

promaty Apr 22, 2020

promaty Apr 22, 2020

promaty Apr 22, 2020

facuspagnuolo Apr 22, 2020

facuspagnuolo Apr 22, 2020

promaty Apr 22, 2020

facuspagnuolo Apr 22, 2020

facuspagnuolo Apr 22, 2020

promaty Apr 22, 2020

facuspagnuolo Apr 22, 2020

facuspagnuolo Apr 22, 2020

promaty Apr 22, 2020

facuspagnuolo Apr 22, 2020

Testing setup #142

Testing setup #142

Conversation

promaty commented Apr 22, 2020 • edited Loading

facuspagnuolo left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

promaty commented Apr 22, 2020 •

edited

Loading