Skip to content

CI: test with pathbase#2031

Open
exyi wants to merge 5 commits into
mainfrom
test-pathbase
Open

CI: test with pathbase#2031
exyi wants to merge 5 commits into
mainfrom
test-pathbase

Conversation

@exyi
Copy link
Copy Markdown
Member

@exyi exyi commented May 22, 2026

No description provided.

Copy link
Copy Markdown

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.

Pull request overview

Adds support for running the ASP.NET Core sample apps (and corresponding UI tests) under a non-root PathBase, to validate DotVVM works correctly when hosted behind a base path.

Changes:

  • Add optional DOTVVM_TEST_PATH_BASE handling in AspNetCoreLatest sample startup to enforce a mandatory base path during CI runs.
  • Extend the UI test GitHub Action to accept a path-base input and pass it through to test scripts.
  • Update the Linux UI test runner script to probe the app under the base path and rewrite seleniumconfig.json base URLs accordingly.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.

File Description
src/Samples/AspNetCoreLatest/Startup.cs Adds middleware to apply/enforce a mandatory PathBase for CI path-base testing.
.github/workflows/main.yml Introduces path-base as a UI test matrix dimension and passes it into the UI test action.
.github/uitest/uitest.sh Updates sample readiness probing and selenium config base URL rewriting to include the optional path base.
.github/uitest/action.yml Adds path-base input and exports it as DOTVVM_TEST_PATH_BASE for both bash and pwsh runs.
Comments suppressed due to low confidence (2)

.github/workflows/main.yml:204

  • The matrix defines path-base, but this include entry doesn’t specify it. In GitHub Actions, included matrix rows can end up without the key, and ${{ matrix.path-base }} later may fail/resolve to null. Add path-base: "" to this and the other include entries that don’t override it.
        path-base: [""]
        include:
          - browser: chrome
            os: windows-2022
            environment: Development

.github/workflows/main.yml:215

  • This include entry (and others below) doesn’t set path-base, but matrix.path-base is referenced later. Ensure every included row sets path-base (typically to "") so the matrix variable is always defined.
          - browser: firefox
            os: ubuntu-latest
            environment: Production
            samples-config: Default
          - browser: firefox

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/Samples/AspNetCoreLatest/Startup.cs Outdated
Comment thread src/Samples/AspNetCoreLatest/Startup.cs
Comment thread .github/uitest/uitest.sh Outdated
Comment on lines +152 to +154
function wait_sample {
PORT=$1
PATH_BASE=$2
@exyi exyi force-pushed the test-pathbase branch 7 times, most recently from 05cb970 to 72e3fad Compare May 26, 2026 22:13
@exyi exyi force-pushed the test-pathbase branch from 72e3fad to f4384fa Compare May 27, 2026 15:44
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