Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Dont work test-script with jest --changedSince #352

Open
CarlFMateus opened this issue Mar 7, 2023 · 17 comments
Open

Dont work test-script with jest --changedSince #352

CarlFMateus opened this issue Mar 7, 2023 · 17 comments
Assignees
Labels
bug Something isn't working

Comments

@CarlFMateus
Copy link

CarlFMateus commented Mar 7, 2023

Describe a bug

The test-script command does not work with this example.

yarn jest --changedSince=origin/master

Because it can't work with this command. Could you help me with this, what would be the best solution. Thank you so much

Expected behavior

For it to work with this type of command, the expected behavior is to perform the tests only for specific files.

Details

  • Action version:

  • OS, where your action is running (windows, linux):

  • action.yml file
    ```yml
    # Insert your action.yml file here. Don't forget to remove all sensitive information (e.g. tokens)
    ```
    
  • Screenshots
    <!-- If you encounter an incorrect coverage comment display, replace this comment with screenshot -->
    
    <!-- If your action unexpectedly fails, please replace this comment with a screenshot of your console  -->
    

Additional context

Captura de Pantalla 2023-03-07 a la(s) 10 29 10 a m

Captura de Pantalla 2023-03-07 a la(s) 10 29 40 a m

@CarlFMateus CarlFMateus added the bug Something isn't working label Mar 7, 2023
@CarlFMateus
Copy link
Author

I would appreciate your help @ArtiomTr

@ArtiomTr
Copy link
Owner

ArtiomTr commented Mar 7, 2023

Hey @CarlFMateus 👋,

Could you please send your action.yml file?

@CarlFMateus
Copy link
Author

CarlFMateus commented Mar 7, 2023

name: Unit Tests
run-name: ${{ github.actor }} is execute test coverage 😎
on:
  pull_request:
    branches:
      - "master"
    types: [opened, reopened, synchronize]

jobs:
  tests:
    if: startsWith(github.head_ref, 'feature')
    runs-on: ubuntu-latest

    permissions:
      id-token: write
      contents: read
      checks: write
      pull-requests: write

    strategy:
      matrix:
        node-version: [16.x]

    steps:
      - name: Checkout
        uses: actions/checkout@v3
        with:
          fetch-depth: 0
      - run: |
          git fetch --no-tags --depth=1 origin master
          git checkout -b master
          git checkout ${{ github.event.pull_request.head.sha }}

      - name: Use Node.js ${{ matrix.node-version }}
        uses: actions/setup-node@v3
        with:
          node-version: ${{matrix.node-version}}
          cache: "yarn"
      - name: Install dependencies
        run: yarn install --frozen-lockfile
      - name: Do you have a unit test?
        uses: actions/github-script@v6
        with:
          github-token: ${{secrets.GITHUB_TOKEN}}
          script: |
            const number = await context.payload.pull_request.number
            const files = await github.rest.pulls.listFiles({
              owner: context.repo.owner,
              repo: context.repo.repo,
              pull_number: number
            })

            const regex = /.*(spec|test).(ts|js|tsx)$/
            const isShow = files.data.some(file => regex.test(file.filename) )

            if(!isShow){
              await github.rest.checks.create({
                owner: context.repo.owner,
                repo: context.repo.repo,
                status: 'completed',
                name: 'Failed Validation',
                head_sha: context.payload.pull_request.head.sha || context.sha,
                conclusion: 'failure',
                output: {
                  title: 'Failed Validation',
                  text: 'Fallo porque no cumplio con las validación minima',
                  summary: 'Debe contener al menos una prueba'
                }
              })
              
              core.setFailed('Debe contener al menos una prueba')
            }

            await github.rest.checks.create({
              owner: context.repo.owner,
              repo: context.repo.repo,
              status: 'completed',
              name: 'Successful Validation',
              head_sha: context.payload.pull_request.head.sha || context.sha,
              conclusion: 'success',
              output: {
                title: 'Successful Validation',
                text: 'Contiene al menos una prueba unitaria',
                summary: 'Debe contener al menos una prueba'
              }
            })
      - name: Coverage Report
        uses: ArtiomTr/jest-coverage-report-action@v2
        with:
          test-script: yarn jest --changedSince=origin/master --coverage
          annotations: failed-tests
          

@CarlFMateus
Copy link
Author

What I try to do is make the coverage only for files that come from the pull request. @ArtiomTr

@ArtiomTr
Copy link
Owner

ArtiomTr commented Mar 7, 2023

@CarlFMateus try this workflow:

name: Unit Tests
run-name: ${{ github.actor }} is execute test coverage 😎
on:
  pull_request:
    branches:
      - "master"
    types: [opened, reopened, synchronize]

jobs:
  tests:
    if: startsWith(github.head_ref, 'feature')
    runs-on: ubuntu-latest

    permissions:
      id-token: write
      contents: read
      checks: write
      pull-requests: write

    strategy:
      matrix:
        node-version: [16.x]

    steps:
      - name: Checkout
        uses: actions/checkout@v3
        with:
          fetch-depth: 0
      - run: |
          git fetch --no-tags --depth=1 origin master
          git checkout -b master
          git checkout ${{ github.event.pull_request.head.sha }}

      - name: Use Node.js ${{ matrix.node-version }}
        uses: actions/setup-node@v3
        with:
          node-version: ${{matrix.node-version}}
          cache: "yarn"
      - name: Install dependencies
        run: yarn install --frozen-lockfile
      - name: Do you have a unit test?
        uses: actions/github-script@v6
        with:
          github-token: ${{secrets.GITHUB_TOKEN}}
          script: |
            const number = await context.payload.pull_request.number
            const files = await github.rest.pulls.listFiles({
              owner: context.repo.owner,
              repo: context.repo.repo,
              pull_number: number
            })

            const regex = /.*(spec|test).(ts|js|tsx)$/
            const isShow = files.data.some(file => regex.test(file.filename) )

            if(!isShow){
              await github.rest.checks.create({
                owner: context.repo.owner,
                repo: context.repo.repo,
                status: 'completed',
                name: 'Failed Validation',
                head_sha: context.payload.pull_request.head.sha || context.sha,
                conclusion: 'failure',
                output: {
                  title: 'Failed Validation',
                  text: 'Fallo porque no cumplio con las validación minima',
                  summary: 'Debe contener al menos una prueba'
                }
              })
              
              core.setFailed('Debe contener al menos una prueba')
            }

            await github.rest.checks.create({
              owner: context.repo.owner,
              repo: context.repo.repo,
              status: 'completed',
              name: 'Successful Validation',
              head_sha: context.payload.pull_request.head.sha || context.sha,
              conclusion: 'success',
              output: {
                title: 'Successful Validation',
                text: 'Contiene al menos una prueba unitaria',
                summary: 'Debe contener al menos una prueba'
              }
            })
      - name: Run tests
        run: yarn jest --changedSince=origin/master --ci --json --coverage --testLocationInResults --outputFile=report.json
      - name: Coverage Report
        uses: ArtiomTr/jest-coverage-report-action@v2
        with:
          coverage-file: report.json
          base-coverage-file: report.json
          annotations: failed-tests       

@CarlFMateus
Copy link
Author

I'm going to try and whatever I keep commenting on this issue, please don't close the issue. Thanks for your help, I'm going to try.

@CarlFMateus
Copy link
Author

Thank you very much if it works, I'll keep testing. Really thank you very much for the help. @ArtiomTr

@CarlFMateus
Copy link
Author

CarlFMateus commented Mar 9, 2023

Sorry, I had to change the code and I'm doing it this way now, executing the jest command in the github script, it runs but doesn't generate the report.json file. Do you suddenly know why this happens? @ArtiomTr I would really appreciate it if you suddenly know the problem, otherwise there is no problem.

name: Unit Tests
#  Esto es una prueba
run-name: ${{ github.actor }} is execute test coverage 😎
on:
  pull_request:
    branches:
      - "master"
    types: [opened, reopened, synchronize]

jobs:
  tests:
    if: startsWith(github.head_ref, 'feature')
    runs-on: ubuntu-latest

    permissions: write-all

    strategy:
      matrix:
        node-version: [16.x]

    steps:
      - uses: actions/checkout@v3
      - name: Use Node.js ${{ matrix.node-version }}
        uses: actions/setup-node@v3
        with:
          node-version: ${{matrix.node-version}}
      - name: Install dependencies
        run: yarn install --frozen-lockfile
      - name: Do you have a unit test?
        uses: actions/github-script@v6
        with:
          script: |
            const number = await context.payload.pull_request.number;
            const files = await github.rest.pulls.listFiles({
              owner: context.repo.owner,
              repo: context.repo.repo,
              pull_number: number,
            });

            const regex = /.*(spec|test).(ts|js|tsx)$/;
            const isShow = files.data.some((file) => regex.test(file.filename));
            if (!isShow) {
              core.setFailed("Debe contener al menos una prueba");
            }

            const cadena = files.data.reduce((acc, element) => {
              if(regex.test(element.filename)) {
                acc.push(element.filename) 
              } 
              return acc
            }, [])

            console.log('cadena', cadena)
            console.log(`jest --config ./jest.config.js ${cadena} --ci --json --coverage --testLocationInResults --outputFile=report.json`)

            const child_process = require('child_process')
            child_process.exec(`jest ${cadena} --ci --json --coverage --testLocationInResults --outputFile=report.json`, (error, stdout, stderr) => {
              if (error) {
                console.log(`error: ${error.message}`);
                return;
              }
              if (stderr) {
                console.log(`stderr: ${stderr}`);
                return;
              }
            
              console.log(`stdout: ${stdout}`);
            })
      - name: Coverage Report
        if: ${{ failure() || success() }}
        uses: ArtiomTr/jest-coverage-report-action@v2
        with:
          coverage-file: report.json
          base-coverage-file: report.json
          annotations: failed-tests

@ArtiomTr
Copy link
Owner

ArtiomTr commented Mar 9, 2023

@CarlFMateus, I don't know why it does not work, but I have a few notices:

  1. You're starting a child process but not waiting until it finishes. Can't tell you if NodeJS can handle this automatically, so try to use child_process.execSync instead.
  2. You're converting an array into a string - by default, it will produce the same output as cadena.join(',') (all values separated by commas). Also, can't tell you if jest can recognize such output. Try to replace it with cadena.join(' '), and see if it works.
  3. Passing just a string into the child_process.exec function can be tricky. This value will be interpreted by the shell, so it won't always do what you want. For instance, if the folder or filename will contain a space, the shell will automatically split it into two different arguments. So it is better to avoid such interpretation, split arguments by yourself and pass them as a second parameter into the child_process.exec function, like so:
    child_process.execSync('jest', [...cadena, '--ci', '--json', '--coverage', '--testLocationInResults', '--outputFile=report.json']);

Also, I've noticed that you're redirecting all output to the console. GitHub already provides a dedicated npm package to tackle this problem - @actions/exec. It also has a really handy promise-based API.

So, I think that something like this should work:

name: Unit Tests
#  Esto es una prueba
run-name: ${{ github.actor }} is execute test coverage 😎
on:
  pull_request:
    branches:
      - "master"
    types: [opened, reopened, synchronize]

jobs:
  tests:
    if: startsWith(github.head_ref, 'feature')
    runs-on: ubuntu-latest

    permissions: write-all

    strategy:
      matrix:
        node-version: [16.x]

    steps:
      - uses: actions/checkout@v3
      - name: Use Node.js ${{ matrix.node-version }}
        uses: actions/setup-node@v3
        with:
          node-version: ${{matrix.node-version}}
      - name: Install dependencies
        run: yarn install --frozen-lockfile
      - name: Do you have a unit test?
        uses: actions/github-script@v6
        with:
          script: |
            const number = await context.payload.pull_request.number;
            const files = await github.rest.pulls.listFiles({
              owner: context.repo.owner,
              repo: context.repo.repo,
              pull_number: number,
            });

            const regex = /.*(spec|test).(ts|js|tsx)$/;
            const isShow = files.data.some((file) => regex.test(file.filename));
            if (!isShow) {
              core.setFailed("Debe contener al menos una prueba");
            }

            const cadena = files.data.reduce((acc, element) => {
              if(regex.test(element.filename)) {
                acc.push(element.filename) 
              } 
              return acc
            }, []);

            // `exec` function is from @actions/exec, it is automatically provided by actions/github-script,
            //    just like `context` or `github` variables.
            await exec.exec('jest', [...cadena, '--ci', '--json', '--coverage', '--testLocationInResults', '--outputFile=report.json']);
      - name: Coverage Report
        if: ${{ failure() || success() }}
        uses: ArtiomTr/jest-coverage-report-action@v2
        with:
          coverage-file: report.json
          base-coverage-file: report.json
          annotations: failed-tests

@CarlFMateus
Copy link
Author

Thank you very much for the help, really thanks for taking the time to analyze the code and find improvements. This interests me a lot since I am implementing your action and I found it very interesting. Thank you very much for the help.

@CarlFMateus
Copy link
Author

Thanks @ArtiomTr for the help, it really worked for me, although if you want I can correct something I found regarding actions/exec. According to the github/exec documentation

await exec.exec('jest', [...cadena, '--ci', '--json', '--coverage', '--testLocationInResults', '--outputFile=report.json']);

Thank you very much for so much help, I just wanted to ask you a favor if you suddenly know. Do you know if with jest you can execute the coverage of multiple files but very specific. I saw that the --collectCoverageFrom flag can be used but an array value will not be sent to the console. I wanted to ask you if you know about this.

This you know would contribute to many developers 😃.

@ArtiomTr
Copy link
Owner

@CarlFMateus, thank you for pointing out a mistake, I don't often use actions/github-script 😄
I will update my comment so that if anyone else faces the same issue, they can copy the working code example.

I don't know precisely how jest CLI handles arrays, but based on my personal experience:

  1. Most commonly, CLI tools concatenate each specified flag value. So if you pass:

    jest --collectCoverageFrom file1 --collectCoverageFrom file2

    CLI will treat this input as ['file1', 'file2']. So possibly, you can modify your code to the following:

    const commonArgs = ['--ci', '--json', '--coverage', '--testLocationInResults', '--outputFile=report.json'];
    const fileArgs = cadena.map(file => `--collectCoverageFrom=${file}`);
    await exec.exec('jest', [...fileArgs, ...commonArgs]);

    (I moved arguments into separate variables, so I don't repeat the same code in the examples below)

  2. Less common, but also popular approach is to handle JSON value. So, for instance:

    jest --collectCoverageFrom "['file1', 'file2']"

    will be parsed as ['file1', 'file2']. So, you could try to modify your code to the:

    // Calling JSON.stringify twice, because if we call once, it will produce a string, without escaped quotes
    const fileArgs = ['--collectCoverageFrom', JSON.stringify(JSON.stringify(cadena))];
  3. Another, similar to the previous one, is when CLI takes comma-separated values. For example:

    jest --collectCoverageFrom file1,file2

    will be parsed as ['file1', 'file2']. If that's the way how jest handles array values, then modify your code to the:

    const fileArgs = ['--collectCoverageFrom', cadena.join(',')];
  4. The last, rather rare, but still occurring array parsing technique, is to parse all values after the first flag specified, and before any next flag. For instance, if you run:

    jest --collectCoverageFrom file1 file2 file3 --ci

    Some CLIs will read the collectCoverageFrom array as ['file1', 'file2', 'file3']. So, if that's the case, try to change your code to the:

    const fileArgs = ['--collectCoverageFrom', ...cadena];

I am pretty sure that one of these variants will work. If not, most likely that jest doesn't allow you to pass arrays as CLI arguments. That would be a great possibility to contribute to a popular tool! 😃

@CarlFMateus
Copy link
Author

Thanks for your help, I did it with the first step that you showed and it worked for me, although it does coverage with more files, it worked for me that way.

Thank you for taking the time to help me and offer some alternatives as soon as I code. 😃

@CarlFMateus
Copy link
Author

CarlFMateus commented Mar 15, 2023

I wanted to ask you, sorry for so much trouble in the code that I'm sharing, I'm adding a flag that is this --config=jest.config.pr.js when playing it locally it takes this flag but when passing it to the coverage report it doesn't take this configuration. Do you know if suddenly it could be a bug? @ArtiomTr. Excuse my English

In this file I am doing the following configuration.

const config = require('./jest.config.js');

module.exports = {
  ...config,
  coverageThreshold: {
    global: {
      branches: 60,
      functions: 30,
      lines: 30,
      statements: 30,
    },
  },
}

Or you would have to add the coverageThreshold in the cli like this:
--coverageThreshold="{\"global\":{\"statements\":30,\"branches\":60}}"

It is taking the metrics from the root file jest.config.js but I need to use the one in the cli --config=jest.config.pr.js

@ArtiomTr
Copy link
Owner

@CarlFMateus, are you having trouble with the jest itself, or with the action? I'm asking because I think in this case, jest works correctly (to ensure that, you can check your action console and see if you get any errors).

Most likely, this is a bug with this action. It is always reading the jest.config.js file for thresholds:

const { config } = await loadConfig({
cwd: workingDirectory,
name: 'jest',
});

(That's just a fancy configuration loader, it will just check jest.config.* with all available extensions)

I'll try to add the option for specifying custom jest configuration.

@CarlFMateus
Copy link
Author

CarlFMateus commented Mar 15, 2023

Following the code that we are seeing in this issue I am trying to add the config flag by specifying this jest.config.pr.js file, it manually generates the report and is sent to the github action of Coverage Report, but it is not taking into account the flag --config=jest.config.pr.js.

Is there a way to use a different configuration file than jest.config.js and how to send it and suddenly I am making a mistake by naming it as jest.config.pr.js?. Would I be wrong to name it as jest.config.pr.js?

If you could establish a different jest configuration to the default configuration, it would help me a lot.

@itsjesusmacias
Copy link

itsjesusmacias commented Nov 7, 2023

Help me 😖 I have the following error:

Captura de pantalla 2023-11-07 a las 21 53 05

This is my config

name: Test project

on:
  pull_request:
    branches:
      - chore/main
  workflow_call:
  workflow_dispatch:

env:
  CI: true

jobs:
  test:
    runs-on: ubuntu-large-4-16
    timeout-minutes: 20
    strategy:
      matrix:
        run-test:
          - XXX
    steps:
      - name: Checkout
        uses: actions/checkout@v3
        with:
          fetch-depth: 0
      - run: |
          git fetch --no-tags --depth=1 origin main
          git checkout -b main
          git checkout ${{ github.event.pull_request.head.sha }}

      - name: Load custom Node version
        id: custom-node
        uses: ./.github/actions/get-node-version

      - name: Use Node.js ${{ steps.custom-node.outputs.node-version }}
        uses: actions/setup-node@v3
        timeout-minutes: 5
        with:
          node-version: ${{ steps.custom-node.outputs.node-version }}

      - name: Install dependencies
        uses: ./.github/actions/install-dependencies
        with:
          ssh-key: ${{ secrets.SSH_KEY }}

      - name: Run tests
        run: cd packages/web && yarn jest --changedSince=origin/main --ci --json --coverage --testLocationInResults --outputFile=report.json
    
      - name: Coverage Report
        uses: ArtiomTr/jest-coverage-report-action@v2
        with:
          coverage-file: report.json
          base-coverage-file: report.json
          annotations: failed-tests    

I have tried using 'main' instead of 'origin/main.' But nothing has worked for me. Can someone help me 🙏🏼?

@ArtiomTr

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants