Skip to content

test: use multiline regex search helper - #3327

Draft
iamdhrv wants to merge 1 commit into
hydra-ecosystem:mainfrom
iamdhrv:agent/multiline-regex-helper
Draft

test: use multiline regex search helper#3327
iamdhrv wants to merge 1 commit into
hydra-ecosystem:mainfrom
iamdhrv:agent/multiline-regex-helper

Conversation

@iamdhrv

@iamdhrv iamdhrv commented Jul 29, 2026

Copy link
Copy Markdown

Change

  • migrate the remaining test call sites from per-line assert_regex_match to
    assert_multiline_regex_search
  • remove the obsolete helper and its line-count/zip implementation

Why

The old helper split both values and ran a separate re.match for every line,
which prevented patterns from expressing multiline context and duplicated the
existing multiline search helper. A single normalized re.search(..., re.MULTILINE) now handles these assertions consistently.

Checks

  • Python byte-compilation passed for all changed modules
  • git diff --check

The focused pytest suite could not start locally because this source checkout
does not contain generated ANTLR parsers and the machine has no Java runtime.
CI's normal parser-generation step remains the authoritative full-suite check.

Fixes #1749

Signed-off-by: Dhruv Maniya <dhruvmaniya1998@gmail.com>
@meta-cla

meta-cla Bot commented Jul 29, 2026

Copy link
Copy Markdown

Hi @iamdhrv!

Thank you for your pull request and welcome to our community.

Action Required

In order to merge any pull request (code, docs, etc.), we require contributors to sign our Contributor License Agreement, and we don't seem to have one on file for you.

Process

In order for us to review and merge your suggested changes, please sign at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need to sign the corporate CLA.

Once the CLA is signed, our tooling will perform checks and validations. Afterwards, the pull request will be tagged with CLA signed. The tagging process may take up to 1 hour after signing. Please give it that time before contacting us about it.

If you have received this in error or have any questions, please contact us at cla@meta.com. Thanks!

@omry

omry commented Jul 29, 2026

Copy link
Copy Markdown
Collaborator

Hi, thanks for your help.
Can you accept the CLA? otherwise I cannot accept this.

@omry omry assigned omry and unassigned omry Jul 30, 2026
@omry omry added the awaiting_response Awaiting response label Jul 30, 2026
@iamdhrv

iamdhrv commented Jul 30, 2026

Copy link
Copy Markdown
Author

I have now signed the individual Meta CLA using this GitHub account. The required check may still be propagating; please let me know if any additional action is needed from me. Thank you.

@omry

omry commented Jul 30, 2026

Copy link
Copy Markdown
Collaborator

@codex, please review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: aa79acab3d

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread tests/test_callbacks.py
result, _err = run_python_script(cmd)

assert_regex_match(
assert_multiline_regex_search(

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Preserve literal matching for expected log output

When any of these callback cases produces the expected output, this assertion still fails because assert_multiline_regex_search always interprets the entire string as a regex. Literal prefixes such as [HYDRA] and [JOB] become character classes, whereas the removed helper first accepted lines that were exactly equal; for example, searching for [HYDRA] Init custom_callback does not match the identical literal text. The same unescaped prefixes occur in the changed sweep and Hydra-mode tests, so either retain the literal-equality behavior or escape those portions before switching helpers.

Useful? React with 👍 / 👎.

@meta-cla meta-cla Bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Jul 30, 2026
@meta-cla

meta-cla Bot commented Jul 30, 2026

Copy link
Copy Markdown

Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Meta Open Source project. Thanks!

@omry

omry commented Aug 5, 2026

Copy link
Copy Markdown
Collaborator

Thanks. Before we can continue reviewing this PR, please:

  1. Rebase the branch onto the latest main. There have been changes in the affected test code since this PR was opened, so the update needs to be evaluated against the current tree.
  2. Address the valid finding in test: use multiline regex search helper #3327 (comment). The old helper accepted exact string equality before treating an expectation as a regex; the replacement always interprets it as a regex, so literal prefixes such as [HYDRA] and [JOB] no longer match themselves. Preserve the equality-first behavior or escape the affected literal expectations.
  3. Add regression coverage for the bracketed literal prefixes and rerun the focused tests/CI.

Once those changes are pushed and CI is clean, please mark the PR ready for review.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

awaiting_response Awaiting response CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

tests: Use multiline regex search instead of assert_regex_match

2 participants