microsoft / fluentui

Fluent UI web represents a collection of utilities, React components, and web components for building web applications.
https://react.fluentui.dev
Other
18.59k stars 2.75k forks source link

App generated by `npm init uifabric` won't run React tests; poor OOBE #12546

Closed petevb closed 3 years ago

petevb commented 4 years ago

It seems like jest isn't configured correctly with the generated app:

Environment Information

Repro

in a clean folder, create, build, & test a new app:

npm init uifabric
npm run build
npm run test

(src\__tests__index.spec.ts will pass)

First problem: Tests in src seem to be ignored, i.e. create an App.spec.tsx test in src & it won't get run unless you tweak that glob/regex in jest.config.js. Change: '<rootDir>/src/**/?(*.)+(spec|test).[tj]s?(x)'] to '<rootDir>/src/**/*.(spec|test).[tj]s?(x)'] it'll then pick up tests in src, but they still won't pass...

Second problem: Create a minimal App.spec.tsx test file in src:

import React from 'react';
import ReactDOM from 'react-dom';
import { App } from './App';

it('renders without crashing', () => {
  const div = document.createElement('div');
  ReactDOM.render(<App />, div);
  ReactDOM.unmountComponentAtNode(div);
});

and re-run npm run test ...

Actual behavior:

> uif2-single@0.1.0 test C:\github\NewOrbit\_test\uif2
> just-scripts test

[19:34:15] ■ started 'test'
[19:34:16] ■ started 'jest'
[19:34:16] ■ Running Jest
[19:34:16] ■ C:\Program Files\nodejs\node.exe 
...
 PASS  src/__tests__/index.spec.ts
 FAIL  src/App.spec.tsx
  ● Test suite failed to run

    ...\src\fabric.png:1
    ({"Object.<anonymous>":function(module,exports,require,__dirname,__filename,global,jest){�PNG

    SyntaxError: Invalid or unexpected token

      2 | import { Stack, Text, Link, FontWeights } from 'office-ui-fabric-react';
      3 | 
    > 4 | import logo from './fabric.png';
        | ^

Expected behavior:

I'd expect the generated app to be able to run the tests out of the box.

Priorities and help requested:

Are you willing to submit a PR to fix? No Requested priority: Normal Products/sites affected: (if applicable): Fluent UI React "Quick Start"

Issue looks similar to #9150 but that was resolved by moving to 7.0.0 so I don't think it's a dupe.

Perhaps the generated app needs moduleNameMapper so jest can handle the png? But I don't see where to do that with the out-of-box just scripts.

As a suggestion, it'd be great if the generated app could include a working component test rather than the simpler index please.

By comparison the Create React App sample at https://github.com/microsoft/create-react-app-uifabric does include an App.test.tsx but for a different App.tsx that doesn't import an image.

Thanks.

joschect commented 4 years ago

@petdun2519 Thanks for the submission, it looks like there are a few points in your issue.

First problem: this seems to be intended behavior, it looks like the jest config that's generated targets the __test__ folder. That said, it would be useful to have this called out in quick start example. We can get that added.

Second problem: This seems like it is likely a bug that I can reproduce. I am uncertain as to why this is not working. We'll take a look and see if there is a good fix!

dzearing commented 4 years ago

@kenotron fyi

petevb commented 4 years ago

Thanks @joschect . As an example fix, if you modify the "minimal" App.test.tsx and import ignore-styles

import React from "react";
import register from 'ignore-styles'
register(['.sass', '.scss', '.css', '.png']);

import { App } from './App';
...

then the test will pass. I think it'd be useful for the generated app to include that npm package and a basic component test as it's a better user experience IMHO.

petevb commented 4 years ago

To the first point, it looks to me like that jest config is trying to run tests that are in __tests__ or src but perhaps I'm misunderstanding. Thanks for the quick response!

joschect commented 4 years ago

@petevb You are correct! I missed the second string in the array. I have been able to run a test in the root folder without issue using both .spec and .test. I'm not sure why you're encountering a problem.

For the second point though, we are actively moving away from using npm init uifabric in favor of using react create app. This pr was one step in moving us away. I'll be changing the documentation on the "Quick Start" page to map to the language used in that pr. In the future we may look into making a RCA configuration to do something similar to what npm init does right now, but that is not currently in the works.

petevb commented 4 years ago

@joschect So does that mean we (uifabric consumers) should move away from the "npm init uifabric created" app and base off CRA instead? We haven't got far, so could start again at this stage. I'm a little nervous to take a "legacy" dependency on just-scripts if the public will be using react-scripts (or ejecting) going forward. Given what you've said, please feel free to close this too. Thanks for the feedback.

petevb commented 4 years ago

Also, please consider exporting this https://github.com/microsoft/fluentui/wiki/Testing#shallowuntiltarget-function as we'll have the same problem as you when trying to write tests for our apps. Let me know if you want a separate issue for that :)

joschect commented 4 years ago

@petevb Yes that's correct. It shouldn't be too much of a difference. and if you want to start with react-create-app is fairly minimal and it should provide a good start! That said I don't believe that just is a legacy package. @kenotron should be able to comment on that.

ShallountilTarget looks like it is exported, but I don't see it as being easily accessible. I'll look into adding it to a better index. That being said, I don't see us actually using it anywhere so I'm going to have to see if it works as expected or if there is another option now.

ecraig12345 commented 3 years ago

Closing this since we've switched to recommending a different starter based on Create React App. See this page for details.