Skip to content
This repository has been archived by the owner on Jul 11, 2022. It is now read-only.

set up unit test #39

Merged
merged 10 commits into from Sep 2, 2020
Merged

set up unit test #39

merged 10 commits into from Sep 2, 2020

Conversation

Nexucis
Copy link
Member

@Nexucis Nexucis commented Aug 28, 2020

I had a lot of trouble to launch the unit test. I'm not really sure about the root cause, but here is the list of the side effect with this PR that doesn't provide so much unit test...

  • The project is fully compatible with esm.
  • I had to change the extension of the webpack config to be compatible
  • Looks like the module @nexucis/autocomplete is not compatible with esm and will need to be updated.
  • Upgrade to node v14. But looking at the build it looks like it works with node v13. No idea if it works with node v12.
    I was personally at the v10 and upgraded to the v12 then v13 and finally to v14 because I had each time multiple different issue
  • Massive changes of the file tsconfig. Just hope that at the end it's still usable when it is packaged ...

Main issue if someone want to do it in a better way: TypeStrong/ts-node#1007

Signed-off-by: Augustin Husson <husson.augustin@gmail.com>
Signed-off-by: Augustin Husson <husson.augustin@gmail.com>
@Nexucis Nexucis requested a review from juliusv August 28, 2020 21:27
Signed-off-by: Augustin Husson <husson.augustin@gmail.com>
@Nexucis Nexucis marked this pull request as draft August 29, 2020 09:27
@Nexucis Nexucis removed the request for review from juliusv August 29, 2020 09:27
@Nexucis
Copy link
Member Author

Nexucis commented Aug 29, 2020

moving as draft since I'm not sure it's a good idea to move to esm because the unit test doesn't work when importing dependencies from CMN.

So far, I'm feeling that esm is really painful and commonJS is still preferable. Maybe I should try another test framework than mocha

@Nexucis
Copy link
Member Author

Nexucis commented Aug 29, 2020

great ... same issue with jest

 FAIL  src/lang-promql/parser/parser.test.ts
  ● Test suite failed to run

    Jest encountered an unexpected token

    This usually means that you are trying to import a file which Jest cannot parse, e.g. it's not plain JavaScript.

    By default, if Jest sees a Babel config, it will use that to transform your files, ignoring "node_modules".

    Here's what you can do:
     • To have some of your "node_modules" files transformed, you can specify a custom "transformIgnorePatterns" in your config.
     • If you need a custom transformation specify a "transform" option in your config.
     • If you simply want to mock your non-JS modules (e.g. binary assets) you can stub them out with the "moduleNameMapper" config option.

    You'll find more details and examples of these config options in the docs:
    https://jestjs.io/docs/en/configuration.html

    Details:

    C:\workspace\perso\test-jest\codemirror-promql\node_modules\@codemirror\next\state\dist\index.js:1
    import { Text, countColumn } from '@codemirror/next/text';
    ^^^^^^

    SyntaxError: Cannot use import statement outside a module

    > 1 | import { EditorState } from '@codemirror/next/state';
        | ^
      2 | import { Parser } from './parser';
      3 | import { ValueType } from './type';
      4 | import { Diagnostic } from '@codemirror/next/lint';

      at Runtime._execModule (node_modules/jest-runtime/build/index.js:1179:56)
      at Object.<anonymous> (src/lang-promql/parser/parser.test.ts:1:1)

Test Suites: 1 failed, 1 total
Tests:       0 total
Snapshots:   0 total
Time:        3.119 s
Ran all test suites.
npm ERR! Test failed.  See above for more details.

jest.config.js

module.exports = {
  transform: { '^.+\\.ts?$': 'ts-jest' },
  testEnvironment: 'node',
  testRegex: '/src/.*\\.test\\.ts$',
  moduleFileExtensions: ['ts', 'tsx', 'js', 'jsx', 'json', 'node'],
};

parser.test.ts

import { EditorState } from '@codemirror/next/state';
import { Parser } from './parser';
import { ValueType } from './type';
import { Diagnostic } from '@codemirror/next/lint';
import { promQLSyntax } from '../promql';

describe('Scalars and scalar-to-scalar operations', () => {
  const testSuites = [
    {
      expr: '1',
      expectedValueType: ValueType.scalar,
      expectedDiag: [] as Diagnostic[],
    },
  ];
  testSuites.forEach((value) => {
    const state = EditorState.create({
      doc: value.expr,
      extensions: promQLSyntax,
    });
    const parser = new Parser(state);
    it(value.expr, () => {
      expect(parser.checkAST(state.tree.firstChild)).toBe(value.expectedValueType);
      expect(parser.getDiagnostics()).toBe(value.expectedDiag);
    });
  });
});

native support for esm in jest: jestjs/jest#4842

I guess there is no possible escape here. Since CMN is a module, then we have to do the same for the test. I just don't get why for the test it needs to be a module too and when we have to build the app, it doesn't matter ....

@juliusv
Copy link
Member

juliusv commented Aug 31, 2020

I'm not an expert in this area at all, so I might not be the best reviewer for this. But 👍 as I cannot complain about anything either :)

Signed-off-by: Augustin Husson <husson.augustin@gmail.com>
Signed-off-by: Augustin Husson <husson.augustin@gmail.com>
Signed-off-by: Augustin Husson <husson.augustin@gmail.com>
@Nexucis
Copy link
Member Author

Nexucis commented Sep 1, 2020

it's ok @juliusv, I managed to understand why it's like that. I still don't like it, but excepting if CMN is ok to move back to commonJS, there is nothing we can do againts it. So we have to deal with it and manage it even if it's a bit painful.

So I will continue to develop some test on this PR and before the end of the week I will merge it.

Signed-off-by: Augustin Husson <husson.augustin@gmail.com>
Signed-off-by: Augustin Husson <husson.augustin@gmail.com>
Signed-off-by: Augustin Husson <husson.augustin@gmail.com>
Signed-off-by: Augustin Husson <husson.augustin@gmail.com>
@Nexucis Nexucis marked this pull request as ready for review September 2, 2020 11:56
@Nexucis Nexucis merged commit ac3db62 into master Sep 2, 2020
@Nexucis Nexucis deleted the feature/test branch September 2, 2020 11:56
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants