avajs / babel

Babel provider for AVA.
11 stars 3 forks source link

External macros are not transformed with power-assert #25

Open MethodGrab opened 8 years ago

MethodGrab commented 8 years ago

Description

External AVA macros are not transformed with power-assert.

Expected

Macros in the same file as the tests (internal macros) should have the same behaviour as those imported from an external file.
Both tests should output the same style of error:

test( 'internalMacro: 2 + 2 === 4', internalMacro, '2 + 2', 8 );
test( 'externalMacro: 2 + 2 === 4', externalMacro, '2 + 2', 8 );

Actual

External macros are not transformed with power-assert and so display standard errors.

Test Source

Full example: https://github.com/MethodGrab/ava-power-assert-external-macro-test-case

test.js:

import test from 'ava';

import { externalMacro } from './macros';

function internalMacro( t, input, expected ) {
    console.log( 'running internal macro...' );
    t.is( eval( input ), expected );
}

// this will output a power-assert error
test( 'internalMacro: 2 + 2 === 4', internalMacro, '2 + 2', 8 );

// this will output a standard assert error
test( 'externalMacro: 2 + 2 === 4', externalMacro, '2 + 2', 8 );

macros.js:

module.exports.externalMacro = function( t, input, expected ) {
    console.log( 'running external macro...' );
    t.is( eval( input ), expected );
}

Error Message & Stack Trace

running internal macro...
  × internalMacro: 2 + 2 === 4
  t.is(eval(input), expected)
       |    |       |
       4    "2 + 2" 8

running external macro...
  × externalMacro: 2 + 2 === 4 4 === 8

  2 tests failed [23:18:50]

  1. internalMacro: 2 + 2 === 4
  AssertionError:
    4    "2 + 2" 8
        Test.internalMacro (test.js:7:4)
        _combinedTickCallback (internal/process/next_tick.js:67:7)
        process._tickCallback (internal/process/next_tick.js:98:9)

  2. externalMacro: 2 + 2 === 4
  AssertionError: 4 === 8
    Test.module.exports.externalMacro (macros.js:3:4)
    _combinedTickCallback (internal/process/next_tick.js:67:7)
    process._tickCallback (internal/process/next_tick.js:98:9)

Config

{
  "ava": {}
}

Command-Line Arguments

ava --verbose test.js

Relevant Links

novemberborn commented 8 years ago

Yes this is definitely an oversight with how we built macros.

@avajs/core / @twada I assume we treat any t.true expression in the test file as if it was a power assertion? I don't think we can scale that to every imported module (recursively!), especially if we'd want to cover macros distributed via npm.

Perhaps we could add an ava/macro helper which compiles the received function source?

sindresorhus commented 8 years ago

I assume we treat any t.true expression in the test file as if it was a power assertion?

Yes

I don't think we can scale that to every imported module (recursively!), especially if we'd want to cover macros distributed via npm.

Correct, mostly because we don't yet transpile imported files. Maybe we could come up with a naming convention for macros so we would know to transpile and power-assert'ify them, kinda like helper files → avajs/ava#720.

Perhaps we could add an ava/macro helper which compiles the received function source?

Can you provide an example of how it would be used?

For now, we should at least find a way to be able to power-assert macros defined in the same file. That's the most common use-case. @twada Any ideas?

twada commented 8 years ago

we should at least find a way to be able to power-assert macros defined in the same file

I think macros in the same file (assume you mean test file) are already power-asserted if the pattern matches (like internalMacro in the PR body).

novemberborn commented 8 years ago

Maybe we could come up with a naming convention for macros so we would know to transpile and power-assert'ify them, kinda like helper files

We could assume helper files contain macros? Intercept the require calls and transpile them separately.

Perhaps we could add an ava/macro helper which compiles the received function source?

Can you provide an example of how it would be used?

// my-macro.js

import macro from 'ava/macro'

export myMacro = macro((t, one, two) => {
  t.true(one === two)
})
// ava/macro.js

export default function (macro) {
  const source = macro.toString()
  return applyPowerAssertAndEvaluate(source)
}

In other words we can get the function source and transpile it on the fly, then evaluate using the vm module. Perhaps with some caching. This would work for macros distributed as dependencies.

sindresorhus commented 8 years ago

We could assume helper files contain macros? Intercept the require calls and transpile them separately.

👍

In other words we can get the function source and transpile it on the fly, then evaluate using the vm module. Perhaps with some caching.

That could work, but I would very much prefer something that just works, like now, than having to have a separate macro function.

novemberborn commented 8 years ago

In other words we can get the function source and transpile it on the fly, then evaluate using the vm module. Perhaps with some caching.

That could work, but I would very much prefer something that just works, like now, than having to have a separate macro function.

Yea, I'm just trying to think about how to distribute macros via npm. I suppose you could distribute version compiled with power-assert and it may just be interoperable.

dashmug commented 8 years ago

Thank you for working on this.

I'm following this thread as I have several macros in my project that I want to reuse in different test files.

sindresorhus commented 8 years ago

@novemberborn I agree that for distributable macros, ava/macro would make sense.

bitjson commented 4 years ago

Have there been any recent developments to this issue with the release of AVA 3?

Does anyone have a preferred workaround for adding power-assert support with imported macros? (Right now for deepEqual I'm stringify-ing the result and expectation in the 3rd message parameter.)

Some additional detail: I'm not importing macros from an external project, I'm simply exporting a macro from a single local "test helper" file, and importing it for use in 5-10 other test files within the same project. The power-assert output works when I run tests for my full project, but when debugging a single test file, no failure output is displayed.

If it's possible to instrument for that scenario – rather than needing to instrument all imported files as mentioned above – or provide some manual way of enabling support for a single file, that would solve my use-case.

novemberborn commented 4 years ago

Have there been any recent developments to this issue with the release of AVA 3?

@bitjson not specifically, except that power-assert now requires the external @ava/babel package. I've moved this issue to that repository.


The power-assert output works when I run tests for my full project, but when debugging a single test file, no failure output is displayed.

Hmm, I can't immediately think of why that would be. We still compile everything when you debug. Could you share a reproduction? This may we worthy of a new issue though.

bitjson commented 4 years ago

Ah, sorry, I thought I was seeing the results of power-assert on test failures, but I never had @ava/babel installed. (My issue is likely unrelated.)

I was actually noticing a difference between when I run all tests (from an independent terminal) and when I debug a single test file in VSCode. In the second case, I don't see any error output from failing macros unless I pass in an assertion message parameter. (I haven't figured out why yet.)