javierbrea / narval Goto Github PK

When you pass an argument to a command that includes a folder separator character, (/), in windows systems it is converted to the OS separator character ().
This occurs because the command path is normalized to the OS correspondants folder separators, but arguments are included.

In the next example, the command "foo-command.sh" will receive the argument $2 as "path\argument2" instead of "path/argument2" in Windows environments. The same effect will occurs with "" characters in Linux environments, which is wrong as well.

suites:
  integration:
    - name: testing
      local:
        command: test/commands/foo-command.sh argument1 path/argument2

It is necessary to normalize only the command, and let the arguments just as are defined in configuration.

For test suites with using the same command for docker and local executions (which is the most common case, if tests are well structured), now it´s needed to define the same command twice. This almost duplicates the number of lines of a configuration file.
To avoid this, the command should be defined at first level of a test suite configuration, and, it could be overwritten for local or docker executions, if needed (as extensions).

This behavior should be applied too for "wait-on" and "env" properties, in all "services" and "before" definitions.
In the case of the "env" property, in could be defined at first level of the suite, and extended with definitions at lower properties levels, as "test", "service", or "before". If it is defined at "docker" or "local" level, it should be extended as well.

When having lots of different suites in a same type of tests, it can be useful to add a describe field to allow identify them easily in configuration and in logs.

suites:
  integration:
    - name: api-books-db
      describe: The api should add books to database
    - name: api-books-db-ko
      describe: The api should return a 500 error if connection to database fails.
    - name: api-logs
      describe: The api should write logs to log file

It is desirable to validate the configuration file, and give feedback about unknown properties that are being ignored, or about mandatory properties that are not defined.

When .narval.yml have too many suites, it's hard to manage that file. The creation of one yaml per suite, or one per type should help with this. Something like that:

suites:
  foo-suite-type:
    - foo-suite:
      services:
        - name: foo-service
          local:
            command: test/commands/start-service.sh
        test:
          specs: test/specs
    - bar-suite:
      services:
        - name: bar-service
          local:
            command: test/commands/start-service.sh
        test:
          specs: test/specs

Can be converted to:

Content of .narval.yml:

suites:
  foo-suite-type:
    - foo-suite: test/foo-suite.yml
    - bar-suite: test/bar-suite.yml

Content of test/foo-suite.yml:

services:
  - name: foo-service
    local:
      command: test/commands/start-service.sh
  test:
    specs: test/specs

Content of test/bar-suite.yml:

services:
  - name: bar-service
    local:
      command: test/commands/start-service.sh
  test:
    specs: test/specs

Allow to disable the "standard" linter using a new configuration property called "enabled". Useful for projects that want to use the tests runner, but another linter.

standard:
    enabled: false

A new version of mocha-sinon-chai has just been released. It is desirable to upgrade the dependency.

In order to set the environment to execute some tests, it would be desirable to be able to execute several commands in before section.

- name: suite-name
  before:
    - name: command-1
      local:
        command: path/to/command1.sh params
    - name: command-2
      local:
        command: path/to/command2.sh params
    - name: command-3
      local:
        command: path/to/command3.sh params

It is needed to add Standard advanced configuration properties that should be "translated" into options for the Standard execution:

standard:
  ignore: # file globs to ignore
    - to-be-ignored/** 
  fix: false # automatically fix problems
  parser: typescript-eslint-parser # js parser
  plugins: # eslint plugins
    - typescript
  envs: # eslint environments
    - browser
  globals: # custom global variables to declare
    - myVar1
    - myVar2

In some scenarios is needed to use the chai assert method, which is not currently available at the Narval interface.
This feature is provided by the new version 2.5.0 of mocha-sinon-chai library.
The mocha-sinon-chai dependency should be upgraded in order to provide this method to Narval interface too.

When running docker suites, and logLevel is "info", the tests execution reports are not displayed.
As they are ran under a docker container, you have to set the log level as "trace" for displaying logs, which is an unexpected behavior, because all services logs are displayed too.

It is desirable that mocha reports inside a docker container were displayed with log level set as "info".

When running a single service or tests locally, the entire .narval/logs folder is being cleaned. This can produce unexpected errors, because another services or test could be running at the same time in another shell, and their logs files will be deleted while executing.

Steps to reproduce it:

1. Execute a single service in a shell, and leave it running:

npm test -- --suite=foo-suite --local=foo-service

2. In another shell, run tests of the same suite:

npm test -- --suite=foo-suite --local=test
# This will delete the logs folder of the previously started service while it´s still running.

The proposed solution consists on cleaning only the log folder of the target service or test, not the full logs folder.

It could be useful to add a new configuration property to standard that will make it non-blocking. This will make easier to refactor progressively packages that are using another linter with different rules.

A report file should be generated with current standard errors, and will be printed to logs, but process will not throw an error.

The file report will be created by default into .narval/reports/standard.log, but this path could be redefined using the new "report" property.

standard:
  report: standard/out.log
  stop-on-error: false

When one service is configured to wait for another to be finished, it produces an error. When the suite execution ends, the full Narval process is finished and it does not execute any other suite.

suites:
  functional:
    - name: foo-suite
      services:
        - name: service-1
          local:
            command: test/commands/foo-command.sh
        - name: service-2
          local:
            command: test/commands/foo-command.sh
            wait-on: .narval/logs/functional/foo-suite/service-1/exit-code.log
      test:
        specs: test/functional/specs
        local:
          wait-on: .narval/logs/functional/foo-suite/service-2/exit-code.log

This example will cause that Narval exit process after execution of suite "foo-suite", which is an error.

The error is caused because for local suites execution, Narval is waiting for all services to be started before attaching to them the respective "on.close" events that, all togueter, handle the suite promise. As one of the service is already closed because the other one waited for him to be finished, the "on.close" is never triggered, and the suite promise is never resolved.

Is necessary to add the "on.close" event for each service just when each one is started.

Providing Standard as built-in linter can be intrusive in packages code styling.
It should be desirable to limit Narval features to tests execution, and let users to implement their own code linters.

In order to make easier the migration of current packages using Narval, it should be great to provide an utility for setting up Standard linter directly in the package. This utility should be removed in next versions, but will allow users to continue using Standard without the need to setup it manually:

Suggestion of command line utility to "eject" Standard from Narval:

npm run test -- --eject-standard

The execution of this command should add the standard linter dependencies to the package, create an .eslintrc file with current Narval Standard settings, create an script in the package.json file to run linter, and display help about how to run the linter from now.

The docker shared volume that is being mounted for tests suites has the same name for all Narval executions in all packages. This can produce conflicts between tests executions of different packages in the same machine.

It is necessary to add a package/version namespace to the shared volume, in order to avoid possible conflicts.

Exists an usual scenario in which you need to wait for a service to have finished before running tests, or starting another service.
To do this, currently you have to define a "wait-on" property pointing to the exit-code.log file of the target service which you are waiting for:

suites:
  foo-suite-type:
    - foo-suite:
      services:
        - name: foo-service
          local:
            command: test/commands/start-service.sh
        test:
          specs: test/specs
          local:
            wait-on: .narval/logs/foo-suite-type/foo-suite/foo-service/exit-code.log

It is desirable to extend the "wait-on" features with a new one specific for Narval:

wait-on: exit:foo-service # This will wait until the service "foo-service" have exited.

Istanbul is crashing when code includes some ES2017 syntax, like "async", or "await":

 Parsing error: Unexpected token async

It seems that the last alpha version of Istanbul supports these javascript features. So, maybe the dependency can be upgraded, assuming the risk of using an alpha version that has not been released after two years (prerelease v1.1.0-alpha.1 is from June, 2016).

Taking into account that maybe it will be necessary to modify Narval in order to use NYC instead of Istanbul at a short term, and the great amount of functional tests that Narval includes, maybe this risk can be assumed.

Anyway, I would like to know the opinion of another Narval collaborators, like @sanguino, @juanmagit or @epibola about this topic before upgrading it.

It can be useful to add new configuration properties called "skip" and "only" that will act as the well known mocha methods with the same name.

It should be possible to add this properties to suites, as in the example:

suites:
  integration:
    - name: suite-1
      only: true
    - name: suite-2
      skip: true

Theses properties should have no effect when command line options "suite" or "type" are used.

In many specs it is desired to read the logs files of a service. There is a very common and basic scenario in which you want to know if a service process has finished with an error or not.

To make this now, you have to "manually" read files like the .narval/logs/[suite-type]/[suite-name]/[service-name]/exit-code.log, for example, which is possible using the process.env.narval_suite_type and process.env.narval_suite environment variables available at the tests execution, but would be great to have a built-in method in narval that provide this feature.

The method could be named "readLogs".

Used in an example:

test = require('narval')

test.describe('The service execution', () => {
  test.it('should have finish OK', () => {
    return test.readLogs('serviceName', 'exit-code')
      .then((log) => {
        return test.expect(log).to.equal('0')
      })
  })
})

It should be great to have a new command line option called "loglevel" that will define logs that are printed to console, or silenced.
This new option should have no effect to services log files, which will continue saving all.

Currently, Narval logs are already categorized into levels "log", "trace", "debug", "info", "warn" and "error". The option will define the level from which logs are printed, (from the beggining of the list). For better comprehesion, read the tracer package documentation

Maybe a review of current assigned levels for each log is required too.

Currently there are many files that are being published to npm, and are not needed for Narval execution, which is increasing unnecessary the weight of the package.

The entire /test folder should be excluded, and sonar configuration file too.

Package should be deprecated. It is desirable to add alternatives to the README file and archive the repository.

Currently it is not possible to add various exclude patterns to coverage exclude option.

This behavior can be achieved adding many "-x" values to the "x" option, but this is only a workaround:

coverage:
  config:
    dir: .coverage
    x: dist/**/*.js -x=webpack.config.js -x=cli/**/*.js

Narval should accept the x option as an array, and internally convert it to various arguments for istanbul. It is desirable too to add an "exclude" alias for the "x" option, to make it more intuitive.

It can be useful to add a new coverage configuration property called "check" that will stop process if the coverage results are less than specified. Internally, the library could use the Istanbul check-coverage command to provide this feature.

The configuration should look like this:

coverage:
  check: #Boolean or Object with advanced config. If Boolean, default config will applied (80)
    statement: 90
    branch: 60
    function: 100
    per-file: true # If true, will check coverage limits for each file.

Current docker logs became very verbose when using environment variables because a lot of warnings are displayed:

WARNING: The mongodb_container_controller_host_name variable is not set. Defaulting to a blank string.
WARNING: The mongodb_container_domapic_path variable is not set. Defaulting to a blank string.
WARNING: The mongodb_container_db_uri variable is not set. Defaulting to a blank string.

This happens because not all environment variables are set for all suites.

When one environment variable is set for one suite, Narval should inject that variable in all suites with an empty string as value to prevent these docker warnings.