Skip to content

envd/tests: ignore ID changes in datadriven ws tests#35510

Merged
teskje merged 1 commit intoMaterializeInc:mainfrom
teskje:ws-datadriven-less-churn
Mar 17, 2026
Merged

envd/tests: ignore ID changes in datadriven ws tests#35510
teskje merged 1 commit intoMaterializeInc:mainfrom
teskje:ws-datadriven-less-churn

Conversation

@teskje
Copy link
Copy Markdown
Contributor

@teskje teskje commented Mar 16, 2026

This is a minimal change to reduce the churn on the datadriven ws tests whenever IDs change in the catalog. Like for timestamps, we simply replace all occurrences of IDs by running a regex.

More flexible approaches, like allowing wildcards for skipping fields have been considered, but likely require changes to the datadriven crate.

Motivation

Discussion in Slack.

@teskje teskje requested a review from a team as a code owner March 16, 2026 17:10
@teskje teskje requested a review from ohbadiah March 16, 2026 17:10
@github-actions
Copy link
Copy Markdown
Contributor

Thanks for opening this PR! Here are a few tips to help make the review process smooth for everyone.

PR title guidelines

  • Use imperative mood: "Fix X" not "Fixed X" or "Fixes X"
  • Be specific: "Fix panic in catalog sync when controller restarts" not "Fix bug" or "Update catalog code"
  • Prefix with area if helpful: compute: , storage: , adapter: , sql:

Pre-merge checklist

  • The PR title is descriptive and will make sense in the git log.
  • This PR has adequate test coverage / QA involvement has been duly considered. (trigger-ci for additional test/nightly runs)
  • If this PR includes major user-facing behavior changes, I have pinged the relevant PM to schedule a changelog post.
  • This PR has an associated up-to-date design doc, is a design doc (template), or is sufficiently small to not require a design.
  • If this PR evolves an existing $T ⇔ Proto$T mapping (possibly in a backwards-incompatible way), then it is tagged with a T-proto label.
  • If this PR will require changes to cloud orchestration or tests, there is a companion cloud PR to account for those changes that is tagged with the release-blocker label (example).

Copy link
Copy Markdown
Contributor

@def- def- left a comment

Choose a reason for hiding this comment

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

Could add a note in doc/developer/guide-testing.md's datadriven section about fixid and also fixtimestamp

Comment thread src/environmentd/tests/server.rs Outdated
@teskje teskje force-pushed the ws-datadriven-less-churn branch from ba5211a to a43b8ee Compare March 17, 2026 09:18
@teskje
Copy link
Copy Markdown
Contributor Author

teskje commented Mar 17, 2026

Could add a note in doc/developer/guide-testing.md's datadriven section about fixid and also fixtimestamp

I don't think that's a good place. Note that the fixtimestamp/fixid flags only apply to a single test in mz-environmentd. They are not available to datadriven tests in general.

@teskje teskje force-pushed the ws-datadriven-less-churn branch from a43b8ee to e0379ca Compare March 17, 2026 10:05
This is a minimal change to reduce the churn on the datadriven ws tests
whenever IDs change in the catalog. Like for timestamps, we simply
replace all occurrences of IDs by running a regex.

More flexible approaches, like allowing wildcards for skipping fields
have been considered, but likely require changes to the `datadriven`
crate.
@teskje teskje force-pushed the ws-datadriven-less-churn branch from e0379ca to c45c02c Compare March 17, 2026 10:17
@teskje teskje requested review from ggevay and removed request for ohbadiah March 17, 2026 10:47
Copy link
Copy Markdown
Contributor

@ggevay ggevay left a comment

Choose a reason for hiding this comment

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

Great, thank you for solving this annoying issue!

@teskje
Copy link
Copy Markdown
Contributor Author

teskje commented Mar 17, 2026

TFTRs!

@teskje teskje merged commit 00a654e into MaterializeInc:main Mar 17, 2026
126 checks passed
@teskje teskje deleted the ws-datadriven-less-churn branch March 17, 2026 10:56
antiguru added a commit to antiguru/materialize that referenced this pull request Mar 18, 2026
Rename `mz_compute_prometheus_metrics` to `mz_cluster_prometheus_metrics`
since the source exposes process-wide metrics, not compute-specific ones.

Fix operator timing: wake at interval boundaries to avoid drift, and
only scrape when the scrape interval has elapsed. Add comment about
data staleness when the scrape interval exceeds the logging interval.

Restore `src/environmentd/tests/testdata/http/ws` to upstream to avoid
reverting the fixid changes from PR MaterializeInc#35510.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
antiguru added a commit to antiguru/materialize that referenced this pull request Mar 18, 2026
Rename `mz_compute_prometheus_metrics` to `mz_cluster_prometheus_metrics`
since the source exposes process-wide metrics, not compute-specific ones.

Fix operator timing: wake at interval boundaries to avoid drift, and
only scrape when the scrape interval has elapsed. Add comment about
data staleness when the scrape interval exceeds the logging interval.

Restore `src/environmentd/tests/testdata/http/ws` to upstream to avoid
reverting the fixid changes from PR MaterializeInc#35510.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
antiguru added a commit to antiguru/materialize that referenced this pull request Mar 18, 2026
Rename `mz_compute_prometheus_metrics` to `mz_cluster_prometheus_metrics`
since the source exposes process-wide metrics, not compute-specific ones.

Fix operator timing: wake at interval boundaries to avoid drift, and
only scrape when the scrape interval has elapsed. Add comment about
data staleness when the scrape interval exceeds the logging interval.

Restore `src/environmentd/tests/testdata/http/ws` to upstream to avoid
reverting the fixid changes from PR MaterializeInc#35510.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
maheshwarip pushed a commit that referenced this pull request Mar 19, 2026
This is a minimal change to reduce the churn on the datadriven ws tests
whenever IDs change in the catalog. Like for timestamps, we simply
replace all occurrences of IDs by running a regex.

More flexible approaches, like allowing wildcards for skipping fields
have been considered, but likely require changes to the `datadriven`
crate.
antiguru pushed a commit to antiguru/materialize that referenced this pull request Mar 26, 2026
…35510)

This is a minimal change to reduce the churn on the datadriven ws tests
whenever IDs change in the catalog. Like for timestamps, we simply
replace all occurrences of IDs by running a regex.

More flexible approaches, like allowing wildcards for skipping fields
have been considered, but likely require changes to the `datadriven`
crate.
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