Update tests to cucumber

[pzavolinsky]

Changes

Replaces the jasmine test runner for cucumber, migrates all tests to a more readable gherkin version.

Rather than having something cryptic like:

  itIs('all', ['./import-star.ts', './import-default.ts'], []);

We now have:

Scenario: Unused export const
  Given file "a.ts" is export const a = 1;
  When analyzing "tsconfig.json"
  Then the result at a.ts is ["a"]

With this in place, writing new tests (in TDD fashion) to cover more cases and/or document bugs, regressions and missing features should be a lot less painful.

More documentation here:

• Gherkin syntax
• picked-cucumber

[mrseanryan]

hi @pzavolinsky the new format is much nicer !

One thing - it seems difficult to see if all the old tests are migrated, as the new format is completely different.

For example I think the 2 tests about --ignorePaths are missing.

Is there some way to make comparing the old vs new a bit easier, for example what about using the same file names as the old tests ?

One other (small) thing - the output has a lot of helper text (Step reference) - seems to be caused by usage: true in Test.ts

Regards, Sean

[pzavolinsky]

we can toggle usage: true based on process.env.USAGE for instance. Then you would run USAGE=1 npm run test.

[pzavolinsky]

Regarding the test migration, you are 100% right, I'm going to manually recheck those. And we could also add nyc to actually check code/branching coverage (with that, we can actually monitor if we are missing a test case).

[mrseanryan]

(related) I found the option ignorePaths (added by me!) was not quite working correctly.

[merged] - https://github.com/pzavolinsky/ts-unused-exports/pull/56

[pzavolinsky]

@mrseanryan I've updated this PR to latest TS and migrated the ---ignorePaths tests. Please take another look and let me know.

Unrlated, I upgraded tslint and added an npm run lint script. There are several linting issues but we should fix them all in a separate PR (ideally just put prettier and forget about it). For now tslint will work, but we should also look into moving to eslint

Cheers!

[mrseanryan]

hi @pzavolinsky looks good and complete, as far as I can see.

We could add 1 more simple test, where 1 export is unused, and 1 unused:

(the original tests seem to have a few of those)

Scenario: One unused export const
  Given file "a.ts" is 
    """
    export const a = 1;
    export const b = 1;
    """
  And file "b.ts" is import { a } from './a';
  When analyzing "tsconfig.json"
  Then the result at a.ts is ["b"]

Could also delete import-spec.js

There are a few @Ignored tests but they look like new ones, so probably can be fixed later.

[mrseanryan]

Assuming app-spec.js is staying, then can delete all data except for:

• data/tsconfig-with-comments.json
• data/tsconfig-include.json

[mrseanryan]

@pzavolinsky if you want, I can help with this PR?

[pzavolinsky]

Assuming app-spec.js is staying...

Actually the new tests are fully equipped to model those tests as well see this one

if you want, I can help with this PR?

That would be great @mrseanryan , please make it your own :) If you could directly add the tests you thing we are missing and also the 2 tests for app-spec.js we could remove the specs dir and npm remove jasmine.

[mrseanryan]

hi @pzavolinsky - have pushed those changes.

I will try to investigate the @ignored tests ...

Sean

[mrseanryan]

@pzavolinsky have fixed the failing tests.

• tested against large codebase (150k lines) - is OK
• added a npm task to run only some tests '@only' which helps for debugging

I think this PR is good to go 😃

[pzavolinsky]

Awesome, I'll merge it in a minute.

added a npm task to run only some tests '@only' which helps for debugging

There is a (somewhat hidden) way to do this with npm run test:unit features/path/to/file.feature (run all tests in that file) and npm run test:unit features/path/to/file.feature:123 (only the test whose Scenario: ... is defined in line 123).

[pzavolinsky]

Merged (3.0.2). Thanks @mrseanryan !

This PR marks a great milestone for us.

From now on, for every bug report (except the "in a huge codebase I get an out-of-memory error") we should be able to write the repro test in a feature before even fixing the code.

We can even accept PRs that only add the repro tests (without the fix) and use that as a guide (like I did with the @ignore ones you just fixed).

If every bug gets at repro scenario in a feature, we can make sure we'll never have a regression on that bug.

I'm a big fan of this type of TDD and I've been using it in my other projects for a while now (e.g. https://github.com/muralco/mural-schema/tree/master/features).

ts-unused-exports was suffering from my busy schedule and lack of attention and it's great to have you onboard Sean!

[mrseanryan]

Thats great, happy to help :)  Sean

Sent from Yahoo Mail on Android

On Mon, 28 Oct 2019 at 18:23, Patricio Zavolinskynotifications@github.com wrote:
Merged (3.0.2). Thanks @mrseanryan !

This PR marks a great milestone for us.

From now on, for every bug report (except the "in a huge codebase I get an out-of-memory error") we should be able to write the repro test in a feature before even fixing the code.

We can even accept PRs that only add the repro tests (without the fix) and use that as a guide (like I did with the @ignore ones you just fixed).

If every bug gets at repro scenario in a featur, we can make sure we'll never have a regression on that bug.

I'm a big fan of this type of TDD and I've been using it in my other projects for a while now (e.g. https://github.com/muralco/mural-schema/tree/master/features).

ts-unused-exports was suffering from my busy schedule and lack of attention and it great to have you onboard Sean!

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or unsubscribe.