mikepenz / action-junit-report

Reports junit test results as GitHub Pull Request Check
https://blog.mikepenz.dev
Apache License 2.0
321 stars 118 forks source link

Trying to understand how glob.create is used in resolvePath #1177

Closed enobrev closed 2 days ago

enobrev commented 1 month ago

Hello,

I started using this yesterday for a project that has a lot of errors. Somewhere around 7k. Obviously that's a serious issue, but that's beyond the scope of this question.

What I found using this action was that it took about 45 minutes to process those 7k errors, and I've narrowed down the reason it takes so long to this line.

  const globber = await glob.create(`**/${normalizedFilename}.*`, {
    followSymbolicLinks: followSymlink
  })

In my case followSymlink is false. Also, in my case, the normalizedFilename is the full path to the tested file (specifically, the github workspace path, but in my local test, I have the full path to the source on my drive outside of the repo path).

What happens is that glob.create() method returns this:

DefaultGlobber {
  patterns: [
    Pattern {
      negate: false,
      segments: [Array],
      trailingSeparator: false,
      searchPath: '/path/to/src/mikepenz/action-junit-report',
      rootRegExp: /\//,
      isImplicitPattern: false,
      minimatch: [Minimatch]
    }
  ],
  searchPaths: [ '/path/to/src/mikepenz/action-junit-report' ],
  options: {
    followSymbolicLinks: false,
    implicitDescendants: true,
    matchDirectories: true,
    omitBrokenSymbolicLinks: true,
    excludeHiddenFiles: false
  }
}

And so when looping through globber.globGenerator(), it never finds the files and so this code basically doesn't do anything, but worse, it's slow. As I said, it takes 45 minutes to process my big terrible failing test result.

When I change it to this:

  const globber = await glob.create(normalizedFilename, {
    followSymbolicLinks: followSymlink
  })

its takes about 1.5 seconds to process my test result.

I had planned to write a PR to resolve this issue, but I don't understand the purpose of this method well enough to know that my adjustments will be the right solution. It's not clear to me why one would search for **/${normalizedFilename}.*.

Here's the test I'm using for this:

# src/test.ts
import {parseTestReports} from './testParser'
(async () => {
  const testResult = await parseTestReports(
    'test',
    '',
    '/path/to/src/mikepenz/action-junit-report/results/test-result.xml',
    '',
    false,
    false,
    [],
    undefined,
    '',
    '',
    [],
    false,
    -1,
    true
  )

  console.log(testResult)
})()

And to run it:

time tsx ./src/test.ts

Unfortunately, I can't share my actual junit result file. But I imagine any junit file will do as long as it has the full path or relative path to the source files.

mikepenz commented 1 month ago

Thank you for the report.

Unfortunately the junit output does not reference and include file path references. as a result, and a fallback solution to be able to reference errors to their source files, the action attempts to find the class files with the glob pattern. As there is no clear path of identifying the file associated with an error this is not the most efficient for sure.

Especially as the globber goes only by normalized file name, and based on the source language this might not be a valid path, so it has to recursively search all places to hopefully find the class file.

it could be an optimisation as you note though to first try and see if there is a direct match. In the case of your usecase, is this applicable?

mikepenz commented 1 month ago

Would it be possible if you run the action in debug and see if the output paths would point directly at the correct files?

  core.debug("Resolving path for ${fileName}")

If that's the case, we might actually also be able to only adjust the glob prefix from **/ to not going from any root.

mikepenz commented 1 month ago

Can you please test v5.0.0-a03 ?

mikepenz commented 2 days ago

Closing due to inactivity