Skip to content

Resolve the Capybara session when the audit runs - #39

Merged
MatheusRich merged 1 commit into
mainfrom
resolve-session-at-audit-time
Sep 23, 2026
Merged

MatheusRich merged 1 commit into
mainfrom
resolve-session-at-audit-time

Conversation

@MatheusRich

Copy link
Copy Markdown
Contributor

The engine builds the auditor inside a before hook. It passes the session that is current at that moment. A host suite can select its driver with driven_by from a before hook of its own. That hook registers later, so it runs later. The auditor then holds the default :rack_test session while the example runs against a real browser. Every audit fails with Capybara::NotSupportedByDriverError on the first visit.

This PR passes the test to the auditor instead of the session. The auditor asks the test for page when it audits, so it always uses the session that is current at that moment. The reporters already receive the test, so the two constructor arguments now match.

The new spec calls driven_by from a plain before hook. It fails on main with the error above and passes with this change.

@MatheusRich

Copy link
Copy Markdown
Contributor Author

@seanpdoyle I've pushed 14a40a1 but looks like some of the failures/flakes are on main already? Should I keep trying to fix this?

@seanpdoyle

Copy link
Copy Markdown
Contributor

@MatheusRich I've merged #42 to pass CI on main. If you rebase, you might have some luck!

@MatheusRich
MatheusRich force-pushed the resolve-session-at-audit-time branch from 14a40a1 to e56a7a5 Compare September 22, 2026 20:34
Comment thread .github/workflows/ci.yml Outdated
Comment on lines +35 to +37
# Chrome sometimes needs more than Ferrum's 10-second default to boot
# on a shared runner.
FERRUM_PROCESS_TIMEOUT: "30"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This was part of a previous PR, and can be omitted.

Suggested change
# Chrome sometimes needs more than Ferrum's 10-second default to boot
# on a shared runner.
FERRUM_PROCESS_TIMEOUT: "30"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Is this coverage possible to achieve from a Minitest System Test? The presence of the RSpec suite is solely for the purpose of exercising the RSpec-facing public interface of the gem.

The rest of the internal test suite is written in Minitest.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I've added a minitest test, but kept this one to also test the RSpec hook ordering thing. LMK if you think this is unnecessary.

@MatheusRich
MatheusRich force-pushed the resolve-session-at-audit-time branch from 9448c0a to 74fa47d Compare September 23, 2026 14:29
The engine builds the auditor inside a `before` hook. It passes the
session that is current at that moment. A host suite can select its
driver with `driven_by` from a `before` hook of its own. That hook
registers later, so it runs later. The auditor then holds the default
`:rack_test` session while the example runs against a real browser.
Every audit fails with `Capybara::NotSupportedByDriverError` on the
first `visit`.

This commit passes the test to the auditor instead of the session.
The auditor asks the test for `page` when it audits, so it always
uses the session that is current at that moment. The reporters
already receive the test, so the two constructor arguments now match.

The new spec calls `driven_by` from a plain `before` hook. It fails
on `main` with the error above and passes with this change.

The new test visits a page inside `using_session`. On `main`, the
auditor audits the blank page of the first session, so the test fails.
It passes with this change.
@MatheusRich
MatheusRich force-pushed the resolve-session-at-audit-time branch from 74fa47d to 5e6cfba Compare September 23, 2026 17:55
@MatheusRich
MatheusRich merged commit abacab3 into main Sep 23, 2026
131 of 132 checks passed
@MatheusRich
MatheusRich deleted the resolve-session-at-audit-time branch September 23, 2026 18:00
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