Add ci workflow #3
Conversation
test/server.test.mjs
Outdated
| }, | ||
| }, | ||
| }, | ||
| }; | ||
|
|
||
| describe('Express Server', function () { |
There was a problem hiding this comment.
This could explain what the test actually does on a higher level?
test/server.test.mjs
Outdated
| @@ -1,8 +1,70 @@ | |||
| import request from 'supertest'; | |||
| import { expect } from 'chai'; | |||
| import app from '../server.js'; | |||
| import config from '../config/config.js'; | |||
|
|
|||
| //Compiler input | |||
There was a problem hiding this comment.
Please remove or make the comment meaningful instead
test/server.test.mjs
Outdated
| }); | ||
| }); | ||
|
|
||
| // Add more tests with the shorter timeout here |
There was a problem hiding this comment.
Stale comment? Or is this a todo?
test/server.test.mjs
Outdated
| sources: { | ||
| 'MyContract.sol': { | ||
| content: ` | ||
| // SPDX-License-Identifier: GPL-3.0 |
There was a problem hiding this comment.
This seems like a bad idea (unless we want to GPL license the whole thing which I doubt?). Not sure if it would hold up legally though as it's just a test fixture.
There was a problem hiding this comment.
this is just fixture, but I can change it, which license would you like to have?
There was a problem hiding this comment.
IANAL but for the fixture either UNLICENSED or whatever this repository license is going sounds like an appropriate choice to me?
| @@ -22,6 +22,7 @@ WORKDIR /app | |||
| COPY package*.json ./ | |||
| RUN npm ci --only=production | |||
| COPY utils/ ./utils | |||
| COPY config/ ./config | |||
There was a problem hiding this comment.
All those copies can just be COPY . . with a correct .dockerignore instead?
There was a problem hiding this comment.
I had this before, but easier is to copy this what is just needed.
There was a problem hiding this comment.
It follows that if we only have one image (i.e. there is only a single Dockerfile), "what is just needed" refers to everything, because what is not needed should be excluded by the .dockerignore anyways.
| @@ -1 +0,0 @@ | |||
| src/public | |||
There was a problem hiding this comment.
Should at least contain node_modules?
There was a problem hiding this comment.
I decided to do not use .dockerignore for now
There was a problem hiding this comment.
Why not? Without a correct .dockerignore, things like node_modules, .git etc. just bloat the build context and lead to unnecessary cache invalidations.
Having a correct .dockerignore is considered a best-pracitce and I think we should stick to that.
Add CI workflow with some tests