Skip to content

Conversation

@dsanders11
Copy link
Contributor

@dsanders11 dsanders11 commented Apr 9, 2025

Ran into the need for this when trying to set up a problem matcher in electron/electron#46577. The issue there is filenames in build errors are relative to a build directory which is outside of the source directory, and there's not enough information in the build error to be able to construct the correct path.

With this change I'll be able to add "fromPath": "src/out/Default/" and everything should just work.

@dsanders11 dsanders11 force-pushed the feat/problem-matchers-default-fromPath branch from e2dafbd to 9917e39 Compare April 10, 2025 04:13
@dsanders11 dsanders11 marked this pull request as ready for review April 10, 2025 04:14
Copilot AI review requested due to automatic review settings April 10, 2025 04:14
@dsanders11 dsanders11 requested a review from a team as a code owner April 10, 2025 04:14
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

@ericsciple
Copy link
Collaborator

ericsciple commented May 2, 2025

End-to-end testing makes sense. Here are the tests I ran through:

Workflow YAML
jobs:
  build:
    runs-on: self-hosted
    permissions:
      id-token: write
    steps:
      - run: |
          echo Create files
          git init .
          mkdir -p dir-level-1/dir-level-2/dir-level-3
          echo "a content" > a.txt
          echo "b content" > dir-level-1/b.txt
          echo "c content" > dir-level-1/dir-level-2/c.txt
          echo "d content" > dir-level-1/dir-level-2/dir-level-3/d.txt
      - run: |
          echo "$MATCHER_JSON" > matcher.json
          echo "::add-matcher::matcher.json"
        env:
          # badFile.js: line 50, col 11, Error - 'myVar' is defined but never used. (no-unused-vars)
          MATCHER_JSON: |
            {
                "problemMatcher": [
                    {
                        "owner": "my-matcher",
                        "pattern": [
                            {
                                "regexp": "^(.+):\\sline\\s(\\d+),\\scol\\s(\\d+),\\s(Error|Warning|Info)\\s-\\s(.+)\\s\\((.+)\\)$",
                                "file": 1,
                                "line": 2,
                                "column": 3,
                                "severity": 4,
                                "message": 5,
                                "code": 6
                            }
                        ]
                    }
                ]
            }
      - run: |
          echo Print errors
          echo 'a.txt: line 1, col 1, Error - First error (e1)'
          echo 'dir-level-1/b.txt: line 1, col 1, Error - Second error (e2)'
          echo 'dir-level-1/dir-level-2/c.txt: line 1, col 1, Error - Third error (e3)'

      ################################################################################
      - run: |
          echo "::remove-matcher owner=my-matcher::"
          echo "$MATCHER_JSON" > matcher.json
          echo "::add-matcher::matcher.json"
        env:
          # badFile.js: line 50, col 11, Error - 'myVar' is defined but never used. (no-unused-vars)
          MATCHER_JSON: |
            {
                "problemMatcher": [
                    {
                        "owner": "my-matcher",
                        "pattern": [
                            {
                                "regexp": "^(.+): (.+):\\sline\\s(\\d+),\\scol\\s(\\d+),\\s(Error|Warning|Info)\\s-\\s(.+)\\s\\((.+)\\)$",
                                "fromPath": 1,
                                "file": 2,
                                "line": 3,
                                "column": 4,
                                "severity": 5,
                                "message": 6,
                                "code": 7
                            }
                        ]
                    }
                ]
            }
      - run: |
          echo Print errors
          echo 'proj.txt: a.txt: line 1, col 1, Error - Fourth error (e4)'
          echo 'dir-level-1/proj.txt: b.txt: line 1, col 1, Error - Fifth error (e5)'
          echo 'dir-level-1/dir-level-2/proj.txt: c.txt: line 1, col 1, Error - Sixth error (e6)'

      ################################################################################
      - run: |
          echo "::remove-matcher owner=my-matcher::"
          echo "$MATCHER_JSON" > matcher.json
          echo "::add-matcher::matcher.json"
        env:
          # badFile.js: line 50, col 11, Error - 'myVar' is defined but never used. (no-unused-vars)
          MATCHER_JSON: |
            {
                "problemMatcher": [
                    {
                        "owner": "my-matcher",
                        "fromPath": "dir-level-1/proj.txt",
                        "pattern": [
                            {
                                "regexp": "^(.+):\\sline\\s(\\d+),\\scol\\s(\\d+),\\s(Error|Warning|Info)\\s-\\s(.+)\\s\\((.+)\\)$",
                                "file": 1,
                                "line": 2,
                                "column": 3,
                                "severity": 4,
                                "message": 5,
                                "code": 6
                            }
                        ]
                    }
                ]
            }
      - run: |
          echo Print errors
          echo 'b.txt: line 1, col 1, Error - Seventh error (e7)'
          echo 'dir-level-2/c.txt: line 1, col 1, Error - Eighth error (e8)'
          echo 'dir-level-2/dir-level-3/d.txt: line 1, col 1, Error - Ninth error (e9)'

@ericsciple ericsciple enabled auto-merge (squash) May 2, 2025 17:23
@ericsciple ericsciple disabled auto-merge May 2, 2025 17:23
@ericsciple
Copy link
Collaborator

FYI - When running through E2E testing, based on the way it currently works, fromPath must be a file path and not a directory path. I think the idea is for example a .csproj filepath printed in the error.

A future enhancement would be for fromPath to be support either a file path or a directory path. But for now I think consistency is ideal.

@ericsciple ericsciple enabled auto-merge (squash) May 2, 2025 20:00
@TingluoHuang TingluoHuang force-pushed the feat/problem-matchers-default-fromPath branch from 9917e39 to 07235fb Compare May 5, 2025 20:37
@ericsciple ericsciple merged commit 1a092a2 into actions:main May 5, 2025
8 checks passed
@dsanders11
Copy link
Contributor Author

A future enhancement would be for fromPath to be support either a file path or a directory path. But for now I think consistency is ideal.

That makes sense, and I can easily use a file path for my particular use case. Thanks!

@dsanders11 dsanders11 deleted the feat/problem-matchers-default-fromPath branch May 5, 2025 22:42
marko-k0 pushed a commit to dfinity/runner that referenced this pull request May 13, 2025
sirredbeard pushed a commit to sirredbeard/runner that referenced this pull request Jun 11, 2025
sirredbeard pushed a commit to sirredbeard/runner that referenced this pull request Jun 11, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants