search

ArnaudBuchholz / ui5-test-runner

A test runner for UI5 applications enabling parallel execution of tests.
https://arnaudbuchholz.github.io/ui5-test-runner/
MIT License
22 stars 9 forks source link

TypeScript files are not excluded from coverage report #103

Closed HymanZHAN closed 1 month ago

HymanZHAN commented 2 months ago

Describe the bug

TypeScript files are not excluded from the coverage report

To Reproduce

  1. Have a project that contains TypeScript test files
  2. Use the following ui5-opa.yaml
ui5-opa.yaml 
specVersion: "3.0"
metadata:
  name: com.sap.calm.imp.lib.ui
  allowSapInternal: true
type: application
server:
  settings:
    httpPort: 3001
    httpsPort: 3001
  customMiddleware:
    # transpile TypeScript to ui5-compliant JavaScript
    - name: ui5-tooling-transpile-middleware
      afterMiddleware: compression
      configuration:
        debug: true
        babelConfig:
          sourceMaps: true
          ignore:
            - "**/*.d.ts"
          presets:
            - - "@babel/preset-env"
              - targets: defaults
            - - transform-ui5
            - "@babel/preset-typescript"
          plugins:
            - istanbul
    - name: "@ui5/middleware-code-coverage"
      afterMiddleware: ui5-tooling-transpile-middleware
      configuration:
        instrument:
          produceSourceMap: true
          coverageGlobalScope: "window.top"
          coverageGlobalScopeFunc: false
        excludePatterns:
          - "resources/"
          - "test"
          - "localService"
          - "DataHandler.js"
          - "typedef/"
          - "dwc-js"
  1. Run OPA tests with ui5-test-runner
  2. See that the TS files inside test folder is still included in coverage report. 
[5] ---------------------------------------|---------|----------|---------|---------|-------------------
[5] File                                   | % Stmts | % Branch | % Funcs | % Lines | Uncovered Line #s 
[5] ---------------------------------------|---------|----------|---------|---------|-------------------
[5] All files                              |   89.42 |    75.19 |   93.36 |   89.44 |                   
[5]  webapp                                |   72.72 |       70 |     100 |   72.72 |                   
[5]   Component.js                         |   72.72 |       70 |     100 |   72.72 | 44-52,59-65       
[5]  webapp/ext/control                    |   62.38 |    41.17 |   56.52 |   62.26 |                   
[5]   CompositeText4Items.js               |   62.38 |    41.17 |   56.52 |   62.26 | ...41,250,256-258 
[5]  webapp/ext/controller                 |   96.91 |    78.84 |   95.55 |   96.91 |                   
[5]   LibraryDetail.controller.js          |    98.4 |    77.77 |   97.05 |    98.4 | 205,302           
[5]   LibraryList.controller.js            |   91.89 |    81.25 |    90.9 |   91.89 | 32,61,74          
[5]  webapp/ext/formatter                  |     100 |      100 |     100 |     100 |                   
[5]   Formatter.js                         |     100 |      100 |     100 |     100 |                   
[5]  webapp/ext/handler                    |   92.17 |    79.56 |   95.45 |   92.17 |                   
[5]   ...urationActivityCreationHandler.js |   97.22 |      100 |     100 |   97.22 | 85                
[5]   DescriptionHandler.js                |     100 |      100 |     100 |     100 |                   
[5]   UploadHandler.js                     |   89.84 |     77.1 |   93.75 |   89.84 | ...45-250,394-397 
[5]  webapp/handler                        |    92.3 |       80 |     100 |    92.3 |                   
[5]   DwcHandler.js                        |    92.3 |       80 |     100 |    92.3 | 42                
[5]  webapp/model                          |   98.43 |    77.77 |     100 |   98.43 |                   
[5]   Constant.js                          |      98 |       75 |     100 |      98 | 43                
[5]   Sorter.js                            |     100 |    57.14 |     100 |     100 | 12-15             
[5]   UnifiedBundle.js                     |     100 |      100 |     100 |     100 |                   
[5]  webapp/test/integration               |   73.33 |       50 |     100 |   73.33 |                   
[5]   opaTests.qunit.ts                    |   73.33 |       50 |     100 |   73.33 | 28-30,65          
[5]  webapp/test/integration/journeys      |     100 |      100 |     100 |     100 |                   
[5]   ConfigurationActivityJourney.ts      |     100 |      100 |     100 |     100 |                   
[5]  webapp/test/integration/pages         |   98.33 |    91.66 |     100 |    98.3 |                   
[5]   ConfigurationActivityDialog.ts       |     100 |      100 |     100 |     100 |                   
[5]   LibraryElementList.ts                |   94.73 |       90 |     100 |   94.44 | 47                
[5]   LibraryElementObjectPage.ts          |     100 |      100 |     100 |     100 |                   
[5]  webapp/test/integration/util          |     100 |      100 |     100 |     100 |                   
[5]   Constant.ts                          |       0 |        0 |       0 |       0 |                   
[5]   Factory.ts                           |     100 |      100 |     100 |     100 |                   
[5]  webapp/util                           |     100 |       75 |     100 |     100 |                   
[5]   Util.js                              |     100 |       75 |     100 |     100 | 53                
[5] ---------------------------------------|---------|----------|---------|---------|-------------------

Notice how other .js files are excluded from webapp/test correctly, but the .ts files are not.

Expected behavior

TS test files should be excluded from the coverage report.

Report

Desktop (please complete the following information):

Additional context

It's probably something that should be addressed on the @ui5/middleware-code-coverage end, but just want to to post here and see if this is actually solvable with some config tweak at the moment. Thanks.

ArnaudBuchholz commented 1 month ago

Hello ! can you share your nyc configuration ?

HymanZHAN commented 1 month ago

(Sorry for the delayed reply due to our public holiday...)

I don't have a specific nyc config. Does ui5-test-runner respect nyc.json though?

Please correct me if I am wrong, but from what I see in the log, the command that generates the report is hardcoded as nyc report --reporter=lcov --reporter=cobertura --reporter=text --temp-dir /SAPDevelop/Work/lib-ui/.nyc_output/merged --report-dir /SAPDevelop/Work/lib-ui/coverage --nycrc-path /SAPDevelop/Work/lib-ui/.nyc_output/settings/nyc.json isn't it?

To add more context, I am running in remote mode with -u, so nyc instrumentation should be skipped.

ArnaudBuchholz commented 1 month ago

Correct, instrumentation is skipped. However, since all executions are done in parallel, nyc is used to aggregate all the collected coverages and it can be configured through nyc.json. Through it, you can tell which files to ignore.

Please see this example : https://github.com/ArnaudBuchholz/ui5-test-runner/blob/main/test/sample.js/nyc.json

HymanZHAN commented 1 month ago

Thanks for the info. I actually tried once, but the result was the same. I'll look into it and give it another try.

What I meant by "hardcoded" is that, from the log, ui5-test-runner is already invoking nyc report to aggregate results with the CLI option --nycrc-path /SAPDevelop/Work/lib-ui/.nyc_output/settings/nyc.json, isn't it? So I don't see how nyc will respect the nyc.json I provide. Or is there some other hidden options that I am not aware of?

ArnaudBuchholz commented 1 month ago

Good question, the nyc.json that is used by the runner is a combination of the one you specify (if any) plus options required by the runner (mostly related to input/output directories)

ArnaudBuchholz commented 1 month ago

Please close the issue if the problem is solved

HymanZHAN commented 1 month ago

sorry got occupied...

The issue pertains. In my project root directory, I have the following nyc.json:

{
    "all": true,
    "sourceMap": false,
    "exclude": ["**/test/**/*.js", "**/test/**/*.ts"]
}

The result:

[5] 04:34 │ Checking remote source files
[5] ──────┴─────────────────────────────
[5] nyc report --reporter=lcov --reporter=cobertura --reporter=text --temp-dir /SAPDevelop/Work/lib-ui/.nyc_output/merged --report-dir /SAPDevelop/Work/lib-ui/coverage --nycrc-path /SAPDevelop/Work/lib-ui/.nyc_output/settings/nyc.json
[5] ---------------------------------------|---------|----------|---------|---------|-------------------
[5] File                                   | % Stmts | % Branch | % Funcs | % Lines | Uncovered Line #s 
[5] ---------------------------------------|---------|----------|---------|---------|-------------------
[5] All files                              |   89.62 |     74.8 |   93.51 |   89.61 |                   
[5]  webapp                                |   72.72 |       70 |     100 |   72.72 |                   
[5]   Component.js                         |   72.72 |       70 |     100 |   72.72 | 44-52,59-65       
[5]  webapp/ext/control                    |   62.38 |    41.17 |   56.52 |   62.26 |                   
[5]   CompositeText4Items.js               |   62.38 |    41.17 |   56.52 |   62.26 | ...41,250,256-258 
[5]  webapp/ext/controller                 |   96.91 |    78.84 |   95.45 |   96.91 |                   
[5]   LibraryDetail.controller.js          |    98.4 |    77.77 |   97.05 |    98.4 | 205,302           
[5]   LibraryList.controller.js            |   91.89 |    81.25 |      90 |   91.89 | 30,59,72          
[5]  webapp/ext/formatter                  |     100 |      100 |     100 |     100 |                   
[5]   Formatter.js                         |     100 |      100 |     100 |     100 |                   
[5]  webapp/ext/handler                    |   91.75 |    78.49 |   95.45 |   91.75 |                   
[5]   ...urationActivityCreationHandler.js |   94.87 |    83.33 |     100 |   94.87 | 86,134            
[5]   DescriptionHandler.js                |     100 |      100 |     100 |     100 |                   
[5]   UploadHandler.js                     |   89.84 |     77.1 |   93.75 |   89.84 | ...45-250,394-397 
[5]  webapp/handler                        |    92.3 |       80 |     100 |    92.3 |                   
[5]   DwcHandler.js                        |    92.3 |       80 |     100 |    92.3 | 42                
[5]  webapp/model                          |   98.43 |    77.77 |     100 |   98.43 |                   
[5]   Constant.js                          |      98 |       75 |     100 |      98 | 43                
[5]   Sorter.js                            |     100 |    57.14 |     100 |     100 | 12-15             
[5]   UnifiedBundle.js                     |     100 |      100 |     100 |     100 |                   
[5]  webapp/test/integration               |   73.33 |       50 |     100 |   73.33 |                   
[5]   opaTests.qunit.ts                    |   73.33 |       50 |     100 |   73.33 | 29-31,67          
[5]  webapp/test/integration/journeys      |     100 |      100 |     100 |     100 |                   
[5]   ConfigurationActivityJourney.ts      |     100 |      100 |     100 |     100 |                   
[5]   ConfigurationRelationJourney.ts      |     100 |      100 |     100 |     100 |                   
[5]  webapp/test/integration/pages         |   98.36 |    91.66 |     100 |   98.33 |                   
[5]   ConfigurationActivityDialog.ts       |     100 |      100 |     100 |     100 |                   
[5]   LibraryElementList.ts                |   94.73 |       90 |     100 |   94.44 | 47                
[5]   LibraryElementObjectPage.ts          |     100 |      100 |     100 |     100 |                   
[5]  webapp/test/integration/util          |     100 |      100 |     100 |     100 |                   
[5]   Constant.ts                          |       0 |        0 |       0 |       0 |                   
[5]   Factory.ts                           |     100 |      100 |     100 |     100 |                   
[5]  webapp/util                           |     100 |       75 |     100 |     100 |                   
[5]   Util.js                              |     100 |       75 |     100 |     100 | 53                
[5] ---------------------------------------|---------|----------|---------|---------|-------------------
HymanZHAN commented 1 month ago

I quickly glanced through nyc's doc and noticed that the accepted config files are

So I tried again after changing the config name to .nycrc.json/.nycrc and it worked in both cases. (It's still a bit weird because when I first tried ui5-test-runner about two months ago, adding a .nycrc didn't help then.)

I will close the issue now, but it would be great if you could double-check the case locally and update the docs if necessary. Looking at the example you linked, when this directory gets tested via npm run test:e2e, the config path is also hardcoded as nyc.json: https://github.com/ArnaudBuchholz/ui5-test-runner/blob/main/test/e2e.js#L201