Add implementation-defined maximums #332
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
apasel422
approved these changes
Dec 4, 2025
Collaborator
apasel422
left a comment
apasel422
left a comment
There was a problem hiding this comment.
The spec and simulator changes here LGTM, though I'll defer to others on the choice of the exact minimum values mandated by the spec.
apasel422
reviewed
Dec 9, 2025
| }, | ||
| "maxConversionSitesPerImpression": 3, | ||
| "maxConversionCallersPerImpression": 3, | ||
| "maxImpressionSitesForConversion": 3, |
Collaborator
There was a problem hiding this comment.
We will have to update impl/e2e.schema.json as well with the new configuration fields.
gracias4mirancho-creator
approved these changes
Dec 26, 2025
Collaborator
|
@martinthomson Just following up on this. |
Co-authored-by: Andrew Paseltiner <apaseltiner@google.com>
Co-authored-by: Andrew Paseltiner <apaseltiner@google.com>
Co-authored-by: Andrew Paseltiner <apaseltiner@google.com>
Some systems may consider `a` to be a registrable domain.
Otherwise, the test may inadvertently be covering the behavior of the invalid `a` domain, rather than the size check.
Co-authored-by: Martin Thomson <mt@lowentropy.net>
Per #189 (comment), who to associate the Save-Impression operation with is unclear in these cases. Support for them could be restored in the future if we hear of use cases and are able to resolve the association question.
To make those tests less dependent on fixed behavior. In the future, we might consider allowing these values to be specified in individual events at a higher level in order to avoid mandating the exact way in which randomness is used.
Co-authored-by: Martin Thomson <mt@lowentropy.net>
Co-authored-by: Martin Thomson <mt@lowentropy.net>
The WebIDL interfaces for AttributionImpressionOptions and AttributionConversionOptions specify multiple fields as either `unsigned long` or `long`, and therefore no e2e test should attempt to pass values outside these ranges, as the associated type conversion (e.g. from JavaScript) is assumed to occur strictly before the API (i.e. the simulator) receives the final values. Enforcing this at the JSON schema level ensures that test suites that import the e2e tests need not concern themselves with the WebIDL-level conversions. The change to the JSON schema revealed one such offending test case in save-impression-errors.json, which attempted to pass `histogramIndex: -1`.
Bumps [qs](https://github.com/ljharb/qs) from 6.14.0 to 6.14.1. - [Changelog](https://github.com/ljharb/qs/blob/main/CHANGELOG.md) - [Commits](ljharb/qs@v6.14.0...v6.14.1) --- updated-dependencies: - dependency-name: qs dependency-version: 6.14.1 dependency-type: indirect ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
* update editors add Ben Case as editor as discussed in editors sync. * Update api.bs
Collaborator
|
Still needs a rebase and conflict resolution. |
Member
Author
|
Should be good now. (You keep moving the target on me.) |
apasel422
approved these changes
Jan 15, 2026
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This includes a bunch of values that I essentially made up. Rationale for that is listed in #174.
We'll have to discuss what appropriate values are
before we can merge this change.
Closes #174.
Preview | Diff