Refactor of ExpressJS files #174
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -6,10 +6,10 @@ on: | |
| - dev | ||
| tags: | ||
| - v* | ||
| pull_request: | ||
| branches: | ||
| - dev | ||
| - master | ||
| jobs: | ||
| itest: | ||
| runs-on: ubuntu-20.04 | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,36 @@ | ||
| name: lint | ||
| on: | ||
| push: | ||
| branches: | ||
| - master | ||
| - dev | ||
| tags: | ||
| - v* | ||
| pull_request: | ||
| branches: | ||
| - dev | ||
| - master | ||
|
|
||
| jobs: | ||
| test: | ||
| runs-on: "ubuntu-latest" | ||
| timeout-minutes: 10 | ||
| strategy: | ||
| matrix: | ||
| node-version: [14.x] | ||
| steps: | ||
| - uses: actions/checkout@v2 | ||
| - name: Use Node.js ${{ matrix.node-version }} | ||
| uses: actions/setup-node@v1 | ||
| with: | ||
| node-version: ${{ matrix.node-version }} | ||
|
|
||
| - name: Install dependencies | ||
| run: npm install | ||
|
|
||
| # This file is required by most controllers and triggers "import/no-unresolved" if missing | ||
| - name: Copy config file | ||
| run: cp ./__tests__/integration/configuration/config-fixture.js ./src/config.js | ||
|
|
||
| - name: Lint | ||
| run: npm run lint | ||
|
Comment on lines
+31
to
+36
There was a problem hiding this comment. Why not just add these steps before testing on the main workflow? There was a problem hiding this comment. Since our tests are currently not green, I tried to create the minimum possible interference with them. So I created a new workflow yml. But now that you mention, I see our wallet mobile follows this practice of running the lint within the I agree that's a good idea, but I would rather make all tests green before making this move. There was a problem hiding this comment. I think the interference caused by linters is not that big. We will be able to differentiate failures caused by them or by the tests. There was a problem hiding this comment. I tried adding the linter to the main workflow on a555448, but it broke when tried to run on Node 8.x, possibly also on 10.x We could use additional actions or configurations to enable the linting only on specific node versions, but I think there is little benefit for this extra complexity. Do you have any suggestions on this? |
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,19 +1,19 @@ | ||
| const config = { | ||
| http_bind_address: 'fakehost', | ||
| http_port: 8001, | ||
| network: 'testnet', | ||
| server: 'http://fakehost:8083/v1a/', | ||
| seeds: { | ||
| stub_seed: | ||
| 'upon tennis increase embark dismiss diamond monitor face magnet jungle scout salute rural master shoulder cry juice jeans radar present close meat antenna mind', | ||
| }, | ||
| multisig: {}, | ||
| tokenUid: '', | ||
| gapLimit: null, | ||
| confirmFirstAddress: null, | ||
| }; | ||
|
|
||
| // Allow change config at runtime | ||
| global.config = config; | ||
|
|
||
| export default config; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unrelated to line
What's your opinion on running the integration tests after unit tests (on the main workflow)?
If the unit tests fail we don't need to run integration.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like the idea of only running the integration tests after the unit ones, but maybe not on the same workflow.
Both tests have a timeout of 40 minutes and generate great amounts of logs. While we're trying to fine tune them, I think it's more helpful to keep them on isolated runs with separate outputs. The cost benefit may not be a relevant trade-off for the isolation of problems while in active development.
I think that with some optimizations on the headless, the tests and the underlying lib we could join those tests in the future. But while we don't get to that point, maybe other solutions like the wait on check action or the
workflow_runproperty could be good alternatives to this ordering of actions.What do you think?