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

Add ci workflow #3

Merged
merged 8 commits into from
Sep 24, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 14 additions & 1 deletion .dockerignore
Original file line number Diff line number Diff line change
@@ -1 +1,14 @@
src/public
Copy link
Member

@xermicus xermicus Sep 23, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should at least contain node_modules?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I decided to do not use .dockerignore for now

Copy link
Member

@xermicus xermicus Sep 23, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

.git
.github
.gitignore
.gitmodules
.prettierrc
artillery.yml
Dockerfile
pod.yaml
README.md
remix-project.patch
node_modules
remix-project
public
test
40 changes: 40 additions & 0 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
name: CI

on:
push:
branches: ["main"]
pull_request:
branches: ["main"]

jobs:
test:
runs-on: ubuntu-latest

steps:
- uses: actions/checkout@v4

- uses: actions/setup-node@v4
with:
node-version: 18
cache: 'npm'

- name: Install packages
run: npm ci

- name: Install solc
run: |
mkdir -p solc
wget https://github.com/ethereum/solidity/releases/download/v0.8.26/solc-static-linux \
-O solc/solc && chmod +x solc/solc
echo "$(pwd)/solc/" >> $GITHUB_PATH

- name: Install resolc
run: |
mkdir -p resolc
wget https://github.com/smiasojed/revive/releases/download/0.0.1/resolc \
-O resolc/resolc && chmod +x resolc/resolc
echo "$(pwd)/resolc/" >> $GITHUB_PATH

- name: Test
run: npm test
timeout-minutes: 5
2 changes: 1 addition & 1 deletion .github/workflows/release-image.yml
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ jobs:
push_to_registry:
name: Push Docker image to Docker Hub
runs-on: ubuntu-latest
environment: master
environment: main
steps:
- name: Check out the repo
uses: actions/checkout@v4
Expand Down
1 change: 1 addition & 0 deletions Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ WORKDIR /app
COPY package*.json ./
RUN npm ci --only=production
COPY utils/ ./utils
COPY config/ ./config
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All those copies can just be COPY . . with a correct .dockerignore instead?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had this before, but easier is to copy this what is just needed.

Copy link
Member

@xermicus xermicus Sep 23, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

COPY server.js ./
RUN chown -R node:node /app

Expand Down
6 changes: 6 additions & 0 deletions config/config.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
module.exports = {
server: {
port: 3000,
compilationTimeout: 10000,
},
};
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
"version": "1.0.0",
"main": "server.js",
"scripts": {
"test": "mocha",
"test": "mocha --exit",
"start": "node server.js",
"format": "prettier --write \"**/*.{js,mjs}\"",
"load-test": "artillery run artillery.yml"
Expand Down
10 changes: 6 additions & 4 deletions server.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,9 @@ const { body, validationResult } = require('express-validator');
const async = require('async');
const os = require('os');
const { getErrorMessage } = require('./utils/errorHandler');
const config = require('./config/config');

const app = express();
const port = 3000;

// Logger function for JSON format
const log = (level, message, meta = {}) => {
Expand Down Expand Up @@ -75,7 +75,9 @@ const numCPUs = os.cpus().length;
// Create an async queue that processes compilation tasks
const queue = async.queue((task, done) => {
const { cmd, input } = task;
const solc = spawn('resolc', [cmd], { timeout: 10 * 1000 });
const solc = spawn('resolc', [cmd], {
timeout: config.server.compilationTimeout,
});
let stdout = '';
let stderr = '';

Expand Down Expand Up @@ -252,8 +254,8 @@ function handleResult(request, response, end, result) {
}

if (require.main === module) {
const server = app.listen(port, () => {
log('info', `solc proxy server listening to ${port}`);
const server = app.listen(config.server.port, () => {
log('info', `solc proxy server listening to ${config.server.port}`);
log('info', `Set number of workers to ${numCPUs}`);
});
server.requestTimeout = 5000;
Expand Down
110 changes: 64 additions & 46 deletions test/server.test.mjs
Original file line number Diff line number Diff line change
@@ -1,8 +1,67 @@
import request from 'supertest';
import { expect } from 'chai';
import app from '../server.js';
import config from '../config/config.js';

const compilerInput = {
language: 'Solidity',
sources: {
'MyContract.sol': {
content: `
// SPDX-License-Identifier: UNLICENSED
pragma solidity ^0.8.0;
contract MyContract {
function greet() public pure returns (string memory) {
return "Hello";
}
}
`,
},
},
settings: {
optimizer: {
enabled: true,
runs: 200,
},
outputSelection: {
'*': {
'*': ['abi'],
},
},
},
};

describe('Revive Remix Backend tests', function () {
describe('Shorter compilation timeout', () => {
const originalCompilationTimeout = config.server.compilationTimeout;

before(() => {
config.server.compilationTimeout = 1; // Set shorter timeout for these tests
});

after(() => {
// Restore the original compilation timeout after the tests
config.server.compilationTimeout = originalCompilationTimeout;
});

it('should return 500 for compiler execution timeout on /solc endpoint', function (done) {
const inputString = JSON.stringify(compilerInput);

request(app)
.post('/solc')
.send({
cmd: '--standard-json',
input: inputString,
})
.end((err, res) => {
if (err) return done(err);

expect(res.status).to.equal(500);
done();
});
});
});

describe('Express Server', function () {
it('should return 200 on /metrics endpoint', function (done) {
request(app).get('/metrics').expect(200, done);
});
Expand All @@ -15,35 +74,6 @@ describe('Express Server', function () {
});

it('should return 200 OK for valid --standard-json cmd on /solc endpoint', function (done) {
const compilerInput = {
language: 'Solidity',
sources: {
'MyContract.sol': {
content: `
// SPDX-License-Identifier: GPL-3.0
pragma solidity ^0.8.0;
contract MyContract {
function greet() public pure returns (string memory) {
return "Hello";
}
}
`,
},
},
settings: {
optimizer: {
enabled: true,
runs: 200,
},
outputSelection: {
'*': {
'*': ['abi'],
},
},
},
};

// Stringify the compiler input as required
const inputString = JSON.stringify(compilerInput);

request(app)
Expand Down Expand Up @@ -75,12 +105,12 @@ describe('Express Server', function () {
});

it('should return 200 OK with missing import error for --standard-json cmd on /solc endpoint', function (done) {
const compilerInput = {
language: 'Solidity',
const compilerInputWithImport = {
...compilerInput,
sources: {
'MyContract.sol': {
content: `
// SPDX-License-Identifier: GPL-3.0
// SPDX-License-Identifier: UNLICENSED
pragma solidity ^0.8.0;
import "hardhat/console.sol";
contract MyContract {
Expand All @@ -91,21 +121,10 @@ describe('Express Server', function () {
`,
},
},
settings: {
optimizer: {
enabled: true,
runs: 200,
},
outputSelection: {
'*': {
'*': ['abi'],
},
},
},
};

// Stringify the compiler input as required
const inputString = JSON.stringify(compilerInput);
const inputString = JSON.stringify(compilerInputWithImport);

request(app)
.post('/solc')
Expand All @@ -116,7 +135,6 @@ describe('Express Server', function () {
.end((err, res) => {
if (err) return done(err);

// Assert status is 200 OK
expect(res.status).to.equal(200);
// Check that the response contains the compiled contract
const response = JSON.parse(res.text);
Expand Down