Skip to content

Conversation

@gbrodman
Copy link
Collaborator

@gbrodman gbrodman commented Jan 13, 2026

The primary annoyance with this is that it means we need (or at least, should) split all tests that use the fee extension into two separate tests -- one that simulates non-prod environments, and one that simulates prod environments. This leads to duplication of many tests but that's fine since this is theoretically temporary.


This change is Reviewable

@gbrodman gbrodman force-pushed the onlyFee10NonProd branch 2 times, most recently from 305964d to c45224d Compare January 13, 2026 20:57
@gbrodman
Copy link
Collaborator Author

the way this works:

Basically, we change the XML loaders so that in a non-production environment they only load the fee 1.0 extension, and in production environments we only load the fee-0.x extensions. This totally messes up all the tests that use XML and load fee extensions.

We split each of the Domain*FlowTest classes that uses the fee extension into two classes -- one that runs against a faked production environment and uses the fee-0.x extensions and the standard test file which runs against the fee-1.0 extension (in the unit test environment, which is the default).

We move all tests that use the old fee extensions into the production-simulating test classes and duplicate each test in the main class using the 1.0 extension. This does add tests in general, though we should be able to remove the production-simulating tests eventually.

Copy link
Member

@CydeWeys CydeWeys left a comment

Choose a reason for hiding this comment

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

Would there have been a way to do this more simply by making the loader UNITTEST-aware? I.e. don't remove the draft versions of the fee extension except in sandbox, and then just ... don't test specifically test that configuration? Just have normal individual tests for each version of the fee extension?

@CydeWeys made 1 comment.
Reviewable status: 0 of 120 files reviewed, 1 unresolved discussion (waiting on @gbrodman).

Copy link
Member

@CydeWeys CydeWeys left a comment

Choose a reason for hiding this comment

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

:lgtm:

@CydeWeys made 1 comment.
Reviewable status: 0 of 134 files reviewed, 3 unresolved discussions (waiting on @gbrodman).

Copy link
Collaborator

@weiminyu weiminyu left a comment

Choose a reason for hiding this comment

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

@weiminyu partially reviewed 134 files and all commit messages.
Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @gbrodman).

@gbrodman gbrodman enabled auto-merge January 14, 2026 16:38
Copy link
Collaborator Author

@gbrodman gbrodman left a comment

Choose a reason for hiding this comment

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

@gbrodman made 1 comment and resolved 1 discussion.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @CydeWeys).

Copy link
Member

@CydeWeys CydeWeys left a comment

Choose a reason for hiding this comment

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

:lgtm:

@CydeWeys made 2 comments and resolved 1 discussion.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @gbrodman).

Copy link
Member

@CydeWeys CydeWeys left a comment

Choose a reason for hiding this comment

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

@CydeWeys resolved 1 discussion.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @gbrodman).

The primary annoyance with this is that it means we need (or at least,
should) split all tests that use the fee extension into two separate
tests -- one that simulates non-prod environments, and one that
simulates prod environments. This leads to duplication of many tests but
that's fine since this is theoretically temporary.
@gbrodman gbrodman requested review from weiminyu and removed request for weiminyu January 14, 2026 19:02
@gbrodman gbrodman added this pull request to the merge queue Jan 14, 2026
Merged via the queue into google:master with commit 22ca4e3 Jan 14, 2026
9 of 10 checks passed
@gbrodman gbrodman deleted the onlyFee10NonProd branch January 14, 2026 20:06
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.

3 participants