Skip to content

refactor: clean up SearchLVTest#3163

Merged
Ziinc merged 5 commits intoLogflare:mainfrom
msmithstubbs:test/clean-up-search-lv-test
Feb 20, 2026
Merged

refactor: clean up SearchLVTest#3163
Ziinc merged 5 commits intoLogflare:mainfrom
msmithstubbs:test/clean-up-search-lv-test

Conversation

@msmithstubbs
Copy link
Copy Markdown
Contributor

@msmithstubbs msmithstubbs commented Feb 16, 2026

Cleans up a few things in SearchLVTest, including feedback from #3145

  • Tidy up assertions, remove unnecessary retry_assert/1
  • Replace sleep/1 with wait_for_render/1 in SearchLVTest. Eliminates around 10 seconds

@msmithstubbs msmithstubbs force-pushed the test/clean-up-search-lv-test branch 6 times, most recently from cc12cc8 to 13d8a43 Compare February 17, 2026 05:15
ConsolidatedSup.stop_pipeline(backend.id)
catch
value -> value
end
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.

This cleanup kept throwing an exception and causing CI to fail.

@msmithstubbs msmithstubbs force-pushed the test/clean-up-search-lv-test branch 2 times, most recently from 7951536 to f31cdad Compare February 18, 2026 03:47
@msmithstubbs msmithstubbs marked this pull request as ready for review February 18, 2026 04:02
@msmithstubbs msmithstubbs changed the title wip: clean up SearchLVTest refactor: clean up SearchLVTest Feb 18, 2026
Copy link
Copy Markdown
Contributor

@Ziinc Ziinc left a comment

Choose a reason for hiding this comment

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

@msmithstubbs can you remove the $callers update? other logic looks good.

def init(args) do
source = Keyword.get(args, :source)
callers = Keyword.get(args, :callers)
Process.put(:"$callers", callers)
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.

not a fan of manipulating the process dictionary directly for this since it is subject to otp internal changes.
don't think we need to update it either since it isn't used in any further logic

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.

Removed this and restored the Sandbox.allow/4 calls

Copy link
Copy Markdown
Contributor

@Ziinc Ziinc left a comment

Choose a reason for hiding this comment

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

as above

@msmithstubbs msmithstubbs force-pushed the test/clean-up-search-lv-test branch from f31cdad to 1f42a6f Compare February 20, 2026 00:06
@msmithstubbs msmithstubbs force-pushed the test/clean-up-search-lv-test branch from 1f42a6f to b3a7b67 Compare February 20, 2026 00:07
@msmithstubbs msmithstubbs marked this pull request as draft February 20, 2026 00:09
@msmithstubbs msmithstubbs deleted the test/clean-up-search-lv-test branch February 20, 2026 00:32
@msmithstubbs msmithstubbs restored the test/clean-up-search-lv-test branch February 20, 2026 00:42
@msmithstubbs msmithstubbs reopened this Feb 20, 2026
@msmithstubbs msmithstubbs marked this pull request as ready for review February 20, 2026 01:37
@Ziinc Ziinc merged commit 30de0fe into Logflare:main Feb 20, 2026
7 checks passed
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