Skip to content

Commit

Permalink
🧹 Snapshot improvements (#383)
Browse files Browse the repository at this point in the history
  • Loading branch information
janjakubnanista authored Feb 9, 2024
1 parent f256086 commit cd89a6f
Show file tree
Hide file tree
Showing 6 changed files with 34 additions and 46 deletions.
21 changes: 4 additions & 17 deletions DEVELOPMENT.md
Original file line number Diff line number Diff line change
Expand Up @@ -158,10 +158,10 @@ By default, `jest` will run in [CI mode](https://jestjs.io/docs/cli#--ci). This
To update the snapshots, you will need to run the tests in local mode (so that the new snapshots are written to your filesystem) and pass the [`--updateSnapshot`](https://jestjs.io/docs/cli#--updatesnapshot) CLI flag to `jest`:

```bash
DOCKER_COMPOSE_RUN_TESTS_TURBO_ARGS="--filter=\!./examples/* -- --updateSnapshot" pnpm test:local
CI=1 DOCKER_COMPOSE_RUN_TESTS_TURBO_ARGS="--filter=\!./examples/* -- --updateSnapshot" pnpm test:local
```

If you encounter any errors coming from existing snapshots that have to do with output formatting (i.e. difference in colored/uncolored output), see the [troubleshooting section below](#troubleshooting--updating-snapshots)
If you encounter any errors coming from existing snapshots that have to do with output formatting (i.e. difference in colored/uncolored output), see the [troubleshooting section below](#troubleshooting--snapshots)

#### Container logs

Expand Down Expand Up @@ -298,12 +298,12 @@ If you see this error, just follow turbo's lead and use:
pnpm dev --concurrency 19
```

#### Problems with snapshots
#### Problems with snapshots <a id="troubleshooting--snapshots"></a>

We use jest snapshots in a lot of places throughout the codebase. When an intentional change to the codebase is made and snapshots need to be updated, there are several ways of doing so:

- Erase the original snapshot file and run the test. The snapshot will be recreated and the diff should only show your expected changes
- Run the tests from within the affected package with `-u` flag. This will update the snapshots.
- Run the tests from within the affected package with `--updateSnapshot` flag. This will update the snapshots.

For some packages the snapshot output depends on environment variables and other factors. For example the `io-devtools` tests for printers have different output based on whether the active shell is`TTY` orwhether the `CI` environment variable is set and non-empty.

Expand All @@ -313,19 +313,6 @@ If you encounter errors when running these tests, just set the environment varia
CI=1 pnpm test
```

#### Problems with snapshot updating <a id="troubleshooting--updating-snapshots"></a>

If snapshots are used in a test that relies on filesystem paths, the situation becomes a bit complex. In long term, we should steer away from this approach as it makes updating snapshots a bit cumbersome.

Should you need to update snapshots for this kind of tests, follow these steps:

- Go to `docker-compose.yaml` and find the `volumes` section of the `tests` service
- Uncomment the line that says `./tests:/app/tests` - this will link the contain & host directories where the snapshots are being written
- Run the tests and pass an `--updateSnapshot` flag to `jest`
- `DOCKER_COMPOSE_RUN_TESTS_TURBO_ARGS="--filter=\!./examples/* -- --updateSnapshots" pnpm test:ci` if you want to run all the tests
- `DOCKER_COMPOSE_RUN_TESTS_TURBO_ARGS="--filter=ua-devtools-evm-hardhat-test -- --updateSnapshot" pnpm test:ci` if you want to only run a specific test suite
- Profit

### Problems compiling with `forge`

If running into issues when compiling or running tests using `forge`, make sure you're on a version newer than `01-05-2023`.
Expand Down
4 changes: 0 additions & 4 deletions docker-compose.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -64,10 +64,6 @@ services:
- DOCKER_COMPOSE_RUN_TESTS_TURBO_ARGS=$DOCKER_COMPOSE_RUN_TESTS_TURBO_ARGS
volumes:
- ./node_modules/.cache/turbo:/app/node_modules/.cache/turbo
# Uncomment these lines if you run into issues with outdated snapshots
#
# See DEVELOPMENT.md for more details on this issue
# - ./tests:/app/tests
# Hardhat has an issue with caching compilers inside a docker container,
# failing with EACCES -13 error, pointing to a permissions issue with the cache folder.
#
Expand Down
18 changes: 11 additions & 7 deletions packages/io-devtools/src/config/loading.ts
Original file line number Diff line number Diff line change
@@ -1,41 +1,45 @@
import { importDefault, isFile, isReadable } from '@/filesystem/filesystem'
import { createModuleLogger } from '@/stdio/logger'
import { printZodErrors } from '@/stdio/printer'
import { resolve } from 'path'
import { z } from 'zod'

export const createConfigLoader =
<TConfig>(schema: z.ZodSchema<TConfig>, logger = createModuleLogger('config loader')) =>
async (path: string): Promise<TConfig> => {
const absolutePath = resolve(path)
logger.verbose(`Resolved config file location for '${path}': '${absolutePath}'`)

// First we check that the config file is indeed there and we can read it
logger.verbose(`Checking config file '${path}' for existence & readability`)
const isConfigReadable = isFile(path) && isReadable(path)
logger.verbose(`Checking config file '${absolutePath}' for existence & readability`)
const isConfigReadable = isFile(absolutePath) && isReadable(absolutePath)
if (!isConfigReadable) {
throw new Error(
`Unable to read config file '${path}'. Check that the file exists and is readable to your terminal user`
)
}

// Keep talking to the user
logger.verbose(`Config file '${path}' exists & is readable`)
logger.verbose(`Config file '${absolutePath}' exists & is readable`)

// Now let's see if we can load the config file
let rawConfig: unknown
try {
logger.verbose(`Loading config file '${path}'`)
logger.verbose(`Loading config file '${absolutePath}'`)

rawConfig = await importDefault(path)
rawConfig = await importDefault(absolutePath)
} catch (error) {
throw new Error(`Unable to read config file '${path}': ${error}`)
}

logger.verbose(`Loaded config file '${path}'`)
logger.verbose(`Loaded config file '${absolutePath}'`)

// It's time to make sure that the config is not malformed
//
// At this stage we are only interested in the shape of the data,
// we are not checking whether the information makes sense (e.g.
// whether there are no missing nodes etc)
logger.verbose(`Validating the structure of config file '${path}'`)
logger.verbose(`Validating the structure of config file '${absolutePath}'`)
const configParseResult = schema.safeParse(rawConfig)
if (configParseResult.success === false) {
const userFriendlyErrors = printZodErrors(configParseResult.error)
Expand Down
3 changes: 1 addition & 2 deletions packages/ua-devtools-evm-hardhat/src/utils/taskHelpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ import {
import { createConfigLoader, printJson } from '@layerzerolabs/io-devtools'
import { createEndpointV2Factory, createExecutorFactory } from '@layerzerolabs/protocol-devtools-evm'
import { OAppOmniGraphHardhat, OAppOmniGraphHardhatSchema } from '@/oapp'
import { resolve } from 'path'
import { OAppOmniGraph } from '@layerzerolabs/ua-devtools'
import { Logger } from '@layerzerolabs/io-devtools'

Expand Down Expand Up @@ -166,7 +165,7 @@ export async function validateAndTransformOappConfig(oappConfigPath: string, log
/**
* At this point we have a correctly typed config in the hardhat format
*/
const hardhatGraph: OAppOmniGraphHardhat = await configLoader(resolve(oappConfigPath))
const hardhatGraph: OAppOmniGraphHardhat = await configLoader(oappConfigPath)
/**
* We'll also print out the whole config for verbose loggers
*/
Expand Down
Original file line number Diff line number Diff line change
@@ -1,26 +1,26 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP

exports[`task lz:oapp:wire with invalid configs should fail if the config file does not exist 1`] = `[Error: Unable to read config file '/app/tests/ua-devtools-evm-hardhat-test/does-not-exist.js'. Check that the file exists and is readable to your terminal user]`;
exports[`task lz:oapp:wire with invalid configs should fail if the config file does not exist 1`] = `[Error: Unable to read config file './does-not-exist.js'. Check that the file exists and is readable to your terminal user]`;

exports[`task lz:oapp:wire with invalid configs should fail if the config file is not a file 1`] = `[Error: Unable to read config file '/app/tests/ua-devtools-evm-hardhat-test/test/task/oapp'. Check that the file exists and is readable to your terminal user]`;
exports[`task lz:oapp:wire with invalid configs should fail if the config file is not a file 1`] = `[Error: Unable to read config file 'test/task/oapp/__data__/configs'. Check that the file exists and is readable to your terminal user]`;

exports[`task lz:oapp:wire with invalid configs should fail if the config file is not a valid JSON or JS file 1`] = `[Error: Unable to read config file '/app/tests/ua-devtools-evm-hardhat-test/README.md': SyntaxError: Unexpected token '<']`;
exports[`task lz:oapp:wire with invalid configs should fail if the config file is not a valid JSON or JS file 1`] = `[Error: Unable to read config file 'README.md': SyntaxError: Unexpected token '<']`;

exports[`task lz:oapp:wire with invalid configs should fail with a malformed JS file (001) 1`] = `
[Error: Config from file '/app/tests/ua-devtools-evm-hardhat-test/test/task/oapp/__data__/configs/invalid.config.001.js' is malformed. Please fix the following errors:
[Error: Config from file 'test/task/oapp/__data__/configs/invalid.config.001.js' is malformed. Please fix the following errors:
Property 'contracts.0.contract': Invalid input
Property 'contracts.1.contract': Invalid input
Property 'connections': Required]
`;

exports[`task lz:oapp:wire with invalid configs should fail with a misconfigured file (001) 1`] = `[Error: Config from file '/app/tests/ua-devtools-evm-hardhat-test/test/task/oapp/__data__/configs/valid.config.misconfigured.001.js' is invalid: AssertionError [ERR_ASSERTION]: Could not find a deployment for contract 'NonExistent']`;
exports[`task lz:oapp:wire with invalid configs should fail with a misconfigured file (001) 1`] = `[Error: Config from file 'test/task/oapp/__data__/configs/valid.config.misconfigured.001.js' is invalid: AssertionError [ERR_ASSERTION]: Could not find a deployment for contract 'NonExistent']`;

exports[`task lz:oapp:wire with invalid configs should fail with an empty JS file 1`] = `
[Error: Config from file '/app/tests/ua-devtools-evm-hardhat-test/test/task/oapp/__data__/configs/invalid.config.empty.js' is malformed. Please fix the following errors:
[Error: Config from file 'test/task/oapp/__data__/configs/invalid.config.empty.js' is malformed. Please fix the following errors:
Property 'contracts': Required
Property 'connections': Required]
`;

exports[`task lz:oapp:wire with invalid configs should fail with an empty JSON file 1`] = `[Error: Unable to read config file '/app/tests/ua-devtools-evm-hardhat-test/test/task/oapp/__data__/configs/invalid.config.empty.json': SyntaxError: Unexpected end of JSON input]`;
exports[`task lz:oapp:wire with invalid configs should fail with an empty JSON file 1`] = `[Error: Unable to read config file 'test/task/oapp/__data__/configs/invalid.config.empty.json': SyntaxError: Unexpected end of JSON input]`;
20 changes: 11 additions & 9 deletions tests/ua-devtools-evm-hardhat-test/test/task/oapp/wire.test.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import hre from 'hardhat'
import { isFile, promptToContinue } from '@layerzerolabs/io-devtools'
import { relative, resolve } from 'path'
import { dirname, join, relative, resolve } from 'path'
import { TASK_LZ_OAPP_WIRE } from '@layerzerolabs/ua-devtools-evm-hardhat'
import { deployOApp } from '../../__utils__/oapp'
import { cwd } from 'process'
Expand All @@ -25,9 +25,9 @@ describe(`task ${TASK_LZ_OAPP_WIRE}`, () => {
const expectTransaction = { data: expect.any(String), point: expectOmniPoint, description: expect.any(String) }
const expectTransactionWithReceipt = { receipt: expect.any(Object), transaction: expectTransaction }

const CONFIGS_BASE_DIR = resolve(__dirname, '__data__', 'configs')
const CONFIGS_BASE_DIR = relative(cwd(), join(__dirname, '__data__', 'configs'))
const configPathFixture = (fileName: string): string => {
const path = resolve(CONFIGS_BASE_DIR, fileName)
const path = join(CONFIGS_BASE_DIR, fileName)

expect(isFile(path)).toBeTruthy()

Expand All @@ -52,15 +52,17 @@ describe(`task ${TASK_LZ_OAPP_WIRE}`, () => {
})

it('should fail if the config file is not a file', async () => {
await expect(hre.run(TASK_LZ_OAPP_WIRE, { oappConfig: __dirname })).rejects.toMatchSnapshot()
const oappConfig = dirname(configPathFixture('invalid.config.empty.json'))

await expect(hre.run(TASK_LZ_OAPP_WIRE, { oappConfig })).rejects.toMatchSnapshot()
})

it('should fail if the config file is not a valid JSON or JS file', async () => {
const readme = resolve(__dirname, '..', '..', '..', 'README.md')
const oappConfig = 'README.md'

expect(isFile(readme)).toBeTruthy()
expect(isFile(oappConfig)).toBeTruthy()

await expect(hre.run(TASK_LZ_OAPP_WIRE, { oappConfig: readme })).rejects.toMatchSnapshot()
await expect(hre.run(TASK_LZ_OAPP_WIRE, { oappConfig })).rejects.toMatchSnapshot()
})

it('should fail with an empty JSON file', async () => {
Expand Down Expand Up @@ -109,9 +111,9 @@ describe(`task ${TASK_LZ_OAPP_WIRE}`, () => {
expect(promptToContinueMock).not.toHaveBeenCalled()
})

it('should work with relative paths', async () => {
it('should work with absolute paths', async () => {
const oappConfigAbsolute = configPathFixture('valid.config.empty.js')
const oappConfig = relative(cwd(), oappConfigAbsolute)
const oappConfig = resolve(oappConfigAbsolute)

await hre.run(TASK_LZ_OAPP_WIRE, { oappConfig })

Expand Down

0 comments on commit cd89a6f

Please sign in to comment.