Skip to content

Wrong line number shown for failing assert #1980

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

Closed
ratiotile opened this issue Nov 14, 2018 · 11 comments
Closed

Wrong line number shown for failing assert #1980

ratiotile opened this issue Nov 14, 2018 · 11 comments
Labels
bug current functionality does not work as desired 💵 Funded on Issuehunt This issue has been funded on Issuehunt help wanted question

Comments

@ratiotile
Copy link

ratiotile commented Nov 14, 2018

Issuehunt badges

Description

AVA reports the wrong line number on failing tests. It seems that sourcemaps aren't working, as the line number is greater than the size of the test file. When the line number falls within the source file, a snippet is displayed, but it does not correspond with the failing assertion.

Test Source

import test from 'ava'
import * as convert from './parameter_convert.js'

test('int converter', t => {
  let converted = convert.to.int('foo')
  t.is(converted, 'foo')
})

Error Message & Stack Trace

The test file is only 7 lines long, yet the line 40 is indicated in the message below:

middleware › test_parameter_convert › int converter

test_parameter_convert.js:40

  Difference:

  - NaN
  + 'foo'

Config

I am using ava.config.js:

export default {
  files: ['src/**/test_*.js'],
  sources: ['src/**/*.js'],
  color: true,
  compileEnhancements: false,
  require: ['./babel_register.js'],
  verbose: true,
}

.babelrc

module.exports = function(api) {
  let config = {
    sourceMaps: 'inline',
    presets: [
      '@babel/preset-flow',
      [
        '@babel/preset-env',
        {
          targets: {
            node: 'current',
          },
          modules: 'commonjs',
        },
      ],
    ],
  api.cache.forever()
  return config
}

Command-Line Arguments

ava

Relevant Links

Minimal reproduction
parameter_convert.js

// @flow
/**
 * Conversion functions
 */
export const to = {
  string: (raw: any) => String(raw),
  date: (raw: any) => new Date(raw),
  int: (raw: any) => {
    const converted = Number(raw)
    return converted === undefined ? raw : converted
  },
}

Environment

darwin 17.7.0
ava 1.0.0-rc.2
npm 6.4.1

There is a $60.00 open bounty on this issue. Add more on Issuehunt.

@ratiotile
Copy link
Author

ratiotile commented Nov 15, 2018

Updated with minimal reproduction
edit: This seems to be a regression. AVA 0.25.0 doesn't have this behavior.
edit2: AVA 0.25.0 shows the correct line for a failing assert, but the wrong line in the code under test.

@novemberborn
Copy link
Member

Could you try configuring @babel/register so it ignores the test files? See the last example in https://github.com/avajs/ava/blob/master/docs/recipes/babel.md#compile-sources.

I suspect the double compilation may be causing trouble. Perhaps AVA loads its internal source map and the one from @babel/register is ignored. Or perhaps both @babel/register and AVA are trying to set up source map handling and it's failing… unfortunately this stuff is really hard to get right.

@ratiotile
Copy link
Author

I followed the docs and tried to configure @babel/register

  1. ignore: ['src/test_*'] doesn't work
  2. ignore: ['src/test_parameter_convert.js'] works. Maybe wildcards are broken?
  3. ignore: [/src\/test_.*\.js/] the ignore items must be regex.

Seems that the docs are wrong about how to configure @babel/register. Would you accept a PR on the docs?

@ratiotile
Copy link
Author

Errors in the source file are still reported on the wrong line (a failing assert in the test file is correctly reported). I tried disabling sourceMaps in babel/register, but it still doesn't work.

@novemberborn
Copy link
Member

  1. ignore: ['src/test_*'] doesn't work

That may need a file extension.

Seems that the docs are wrong about how to configure @babel/register. Would you accept a PR on the docs?

Always!

Errors in the source file are still reported on the wrong line (a failing assert in the test file is correctly reported). I tried disabling sourceMaps in babel/register, but it still doesn't work.

OK. It may be a little while until I have time to look into this further.

@ratiotile
Copy link
Author

The workaround I've found is to disable AVA's builtin babel transpilation with compileEnhancements: false, babel: false and rely on babel/register to do it by with the configuration:

require('@babel/register')({
  ignore: [
    /node_modules/,
    /build\//,
    /coverage\//,
  ],
  inputSourceMap: true,
  plugins: [
    [
      'istanbul',
      {
        all: true,
        cache: true,
        exclude: ['**/test_*.js', 'src/functional_tests/*'],
        include: ['src/**/*.js'],
      },
    ],
  ],
  presets: ['@ava/transform-test-files'],
})

@babel/register requires that the ignore paths are regex expressions and not strings.

Now the reported line numbers of errors are correct both in test files and source files.

@klarkc
Copy link

klarkc commented Mar 22, 2019

Happening to me with this config:

{
"ava": {
    "require": [
      "esm"
    ],
    "babel": false,
    "extensions": [
      "mjs"
    ]
  },
}

@kaelzhang
Copy link

kaelzhang commented Apr 22, 2019

The issue always reproduces when package.json.ava.babel: false that I use in every project, which drives me mad.

@sindresorhus sindresorhus added bug current functionality does not work as desired help wanted labels May 2, 2019
@issuehunt-oss issuehunt-oss bot added the 💵 Funded on Issuehunt This issue has been funded on Issuehunt label May 15, 2019
@IssueHuntBot
Copy link

@IssueHunt has funded $60.00 to this issue.


@razvangherghina
Copy link

razvangherghina commented Sep 26, 2019

Just add retainLines: true in babel configuration under sourceMaps: 'inline' and the correct line is shown. no PR added :).

@novemberborn
Copy link
Member

I think this is a source map problem. #2474 could be a solution.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug current functionality does not work as desired 💵 Funded on Issuehunt This issue has been funded on Issuehunt help wanted question
Projects
None yet
Development

No branches or pull requests

7 participants