Skip to content

MOD-9774: Stop Compiling Redis-With Coverage#6231

Merged
kei-nan merged 2 commits intomasterfrom
master_jk_stop_building_redis_with_coverage
Jun 2, 2025
Merged

MOD-9774: Stop Compiling Redis-With Coverage#6231
kei-nan merged 2 commits intomasterfrom
master_jk_stop_building_redis_with_coverage

Conversation

@kei-nan
Copy link
Collaborator

@kei-nan kei-nan commented May 29, 2025

Due to a bug in redis which was fixed in unstable but not in 8.0 exiting from a forked child process which was compiled in coverage can deadlock.
This can lead to failures in the coverage CI flow.

For now, we decided to disable the coverage when compiling the redis server.
Also bringing the coverage back as requirement

A clear and concise description of what the PR is solving, including:

  1. Current: We compile redis with coverage
  2. Change: Stop compiling redis with coverage
  3. Outcome: Coverage CI should pass since forked child should not deadlock on exit.

Main objects this PR modified

  1. Workflows

Mark if applicable

  • This PR introduces API changes
  • This PR introduces serialization changes

@kei-nan kei-nan self-assigned this May 29, 2025
@kei-nan kei-nan changed the title Stop Compiling Redis-With Coverage MOD-9774: Stop Compiling Redis-With Coverage May 29, 2025
@kei-nan kei-nan requested a review from GuyAv46 May 29, 2025 07:19
@codecov
Copy link

codecov bot commented May 29, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 88.04%. Comparing base (609005d) to head (9246bd5).
Report is 2 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #6231      +/-   ##
==========================================
- Coverage   88.05%   88.04%   -0.02%     
==========================================
  Files         233      233              
  Lines       39914    39914              
  Branches     2881     2881              
==========================================
- Hits        35147    35141       -6     
- Misses       4733     4739       +6     
  Partials       34       34              
Flag Coverage Δ
flow 81.96% <ø> (-0.04%) ⬇️
unit 45.42% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@kei-nan kei-nan added this pull request to the merge queue May 29, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks May 29, 2025
@kei-nan kei-nan added this pull request to the merge queue May 29, 2025
@kei-nan kei-nan removed this pull request from the merge queue due to a manual request May 29, 2025
@kei-nan kei-nan added this pull request to the merge queue May 29, 2025
@kei-nan kei-nan removed this pull request from the merge queue due to a manual request May 29, 2025
@kei-nan kei-nan added this pull request to the merge queue Jun 1, 2025
@kei-nan kei-nan removed this pull request from the merge queue due to a manual request Jun 1, 2025
@kei-nan kei-nan added this pull request to the merge queue Jun 2, 2025
Merged via the queue into master with commit 827a0af Jun 2, 2025
14 checks passed
@kei-nan kei-nan deleted the master_jk_stop_building_redis_with_coverage branch June 2, 2025 13:27
nafraf pushed a commit that referenced this pull request Jun 2, 2025
* stop compiling redis-server with coverage

* bring back coverage as requirement
github-merge-queue bot pushed a commit that referenced this pull request Jun 16, 2025
* Normalize multi-byte char terms

* Fix rm_strdupcase_utf8

* Remove debug log

* Add Russian alphabet and diacritic tests

* Don't use DefaultNormalize_utf8() for chinese

* print available locales

* Check locale before using utf8 normalization

* Fix DefaultNormalize()

* Skip multibyte tests if 'en_US.UTF-8' locale is not available

* Test multibyte stopwords

* Revert changes in install_script.sh

* run sanitizer using ubuntu:latest

* Test_cn: fix language_field

* Remove unused strtolower function from misc.c and misc.h

* Fixes from code review

* Enhance multibyte character tests

* Validate queries using multi-byte stopwords

* Add MULTIBYTE_CHARS config param and install locales

* Use setlocale instead of querylocale for better compatibility

* revert changes in event-pull_request.yml and install French locale for debian/ubuntu

* Convert to lowercase using nunicode library

* nunicode_tolower() returns zero terminated string

* Refactor nunicode_tolower() to improve readability

* Refactor nunicode_tolower() to use destination buffer and improve memory management

* Remove null termination in nunicode_tolower()

* Improve documentation

* Support multi-byte chars for tags

* Support multi-byte chars for synonyms

* unicode_tolower(): avoid duplicating encoded input

* Increase SSO_MAX_LENGTH

* Update test to  use FT.DEBUG DUMP_TAGIDX instead of FT.TAGVALS

* Keep previous test_cn:testSynonym and test_cn:testMixedHighlight

* Update CMake configuration for consistent multi-byte char sorting

* Test JSON index

* Fix JSON test and remove unneeded tolower()

* Fix query_EvalSingleTagNode()

* Refactor string normalization to use case folding instead of lowercase conversion

* Refactor tests to use run_command_on_all_shards for setting DEFAULT_DIALECT

* BWC: Add function to convert string to single codepoint folded runes

* Rename rm_strdupcase() to rm_normalize()

* Add stemming test

* Refactor string normalization to use lowercase transformation instead of folding.
Modify FT.SUGGET flow to be BWC

* Refactor filtering functions to use a transformation callback for rune processing. Remove dead code.

* Update testDFAFilter to use strToSingleCodepointFoldedRunes for rune processing

* Refactor string normalization to use case folding (single codepoint) instead of lowercase transformation

* Revert "Refactor string normalization to use case folding (single codepoint) instead of lowercase transformation"

This reverts commit ebcf994.

* Add case sensitivity option to tag string processing functions

* Rename tag_strtofold to tag_strtolower for clarity and update related references to reflect lowercase transformation

* Simplify length checks in tag_strtolower and rm_normalize functions

* Add documentation after review

* WIP: Enhance unicode_tolower function to handle memory allocation for transformed strings

* WIP: Fixing unicode_tolower usage

* Fix tag_strtolower() to support memory realloc

* skip testToLowerSize() on cluster

* Fix rm_normalize and add more tests

* Fix memory leak

* Fix memory leak 2

* MOD-9609: Pass Private Data Context Back To UnblockClient API (#6237)

* Apparently private data had to be sent back when unblocking the client in order for the free function to get called

* code review comment

* solve memory leak

* MOD-9774: Stop Compiling Redis-With Coverage (#6231)

* stop compiling redis-server with coverage

* bring back coverage as requirement

* Add unit test for unicode_tolower

* Fix memory management in UnicodeToLowerTest and update expected results in multibyte character tests

* Add tests for unsupported Unicode characters in tolower conversion.

* MOD-9768: Add Rust abstractions for Buffer* (#6175)

* Add Rust abstractions for Buffer*

* Add tests

* Make Buffer fields pub

For consistency with BufferReader and BufferWriter. Also there is a very
good chance of us needing to use them directly in the near future.

* Expand the docs a bit more

* Add and improve upon safety comments

* fmt fix

* Add forgotten license headers

* Revert "Make Buffer fields pub"

This reverts commit 1243590.

* Address review comments

* Address review comments

* Better document safety of `Buffer` fields

* Apply suggestions from code review

Co-authored-by: Luca Palmieri <[email protected]>

* Remove an unneeded import

* Use reallocation in mock `Buffer_Grow`

* Satisfy MIRI about some pointer access

* Drop unneeded dep

* Remove unnecessary #includes

* Split FFI into a separate crate

* Add buffer.h API to ffi crate

* Buffer API now based on generated bindings for the C Buffer API

* CI: Set `CARGO_PROFILE_OPTIMISED_TEST_BUILD_OVERRIDE_DEBUG` env

Just acting on the advice from a panic:

```
To improve backtraces for build dependencies, set the CARGO_PROFILE_OPTIMISED_TEST_BUILD_OVERRIDE_DEBUG=true environment variable to enable debug information generation.
```

* Drop linking of internal libs in ffi crate

* Apply suggestions from code review

Co-authored-by: Luca Palmieri <[email protected]>

* Revert "CI: Set `CARGO_PROFILE_OPTIMISED_TEST_BUILD_OVERRIDE_DEBUG` env"

This reverts commit 9bd1d87.

* Drop a redundant import

* Add a few debug_asserts

* Bring back an accidentally deleted assertion in a test

* Fix a .gitignore file

* Make a safety assumption explicit

* Dont build `buffer` crate as static lib

* Don't build bindings for oxidized triemap code

* Make use of BufferWritter::buffer_mut

* Fix build on Alpine

---------

Co-authored-by: Luca Palmieri <[email protected]>

* refactor EmptyIterator (#6240)

* refactor EmptyIterator

* fix useless imports and doc

* update empty_iterator.c to add newline

* add gcc11:bullseye to platforms [MOD-9529] (#6198)

* add gcc11:bullseye to platforms

* add naming

* remove deps

* add nightly platforms option

* set merge queue to false

* fix flow

* fix flow

* chore: Improve granularity of build invalidation logic for FFI bindings (#6244)

* Add `tests/deps/RedisJSON` to `.gitignore` (#6249)

* Skip tests of unsupported unicode 9.0.0 chars

* Fix mem leak 3

* Test stopwords which require memory reallocation

* unicode_tolower update out_len always

* Refactor unicode_tolower function to receive a single  *inout_len argument

* Update documentation for tag_strtolower()

* Fixes after review

* tokenizeTagString: Rename origLen as len

* Test prefix/infix/suffix search with supported unicode chars

* Update test description

* Fix testToLowerConversionExactMatch

* testTextToLowerConversionSimilarMatch: remove unused doc_id in queries

* fix crash in test

* Fix test failure on focal and bullseye: avoid nested f-strings with curly brackets

* Fix test failure on mac: invalid escape sequence

* Use SSO in runesToStr and strToLowerRunes

* Add utf8_len arg to strToLowerRunes()

* Align and move strToLowerRunes comment to header

---------

Co-authored-by: kei-nan <[email protected]>
Co-authored-by: Zeeshan Ali Khan <[email protected]>
Co-authored-by: Luca Palmieri <[email protected]>
Co-authored-by: Joan Fontanals <[email protected]>
Co-authored-by: BenGoldberger <[email protected]>
Co-authored-by: oshadmi <[email protected]>
redisearch-backport-pull-request bot pushed a commit that referenced this pull request Jun 16, 2025
* Normalize multi-byte char terms

* Fix rm_strdupcase_utf8

* Remove debug log

* Add Russian alphabet and diacritic tests

* Don't use DefaultNormalize_utf8() for chinese

* print available locales

* Check locale before using utf8 normalization

* Fix DefaultNormalize()

* Skip multibyte tests if 'en_US.UTF-8' locale is not available

* Test multibyte stopwords

* Revert changes in install_script.sh

* run sanitizer using ubuntu:latest

* Test_cn: fix language_field

* Remove unused strtolower function from misc.c and misc.h

* Fixes from code review

* Enhance multibyte character tests

* Validate queries using multi-byte stopwords

* Add MULTIBYTE_CHARS config param and install locales

* Use setlocale instead of querylocale for better compatibility

* revert changes in event-pull_request.yml and install French locale for debian/ubuntu

* Convert to lowercase using nunicode library

* nunicode_tolower() returns zero terminated string

* Refactor nunicode_tolower() to improve readability

* Refactor nunicode_tolower() to use destination buffer and improve memory management

* Remove null termination in nunicode_tolower()

* Improve documentation

* Support multi-byte chars for tags

* Support multi-byte chars for synonyms

* unicode_tolower(): avoid duplicating encoded input

* Increase SSO_MAX_LENGTH

* Update test to  use FT.DEBUG DUMP_TAGIDX instead of FT.TAGVALS

* Keep previous test_cn:testSynonym and test_cn:testMixedHighlight

* Update CMake configuration for consistent multi-byte char sorting

* Test JSON index

* Fix JSON test and remove unneeded tolower()

* Fix query_EvalSingleTagNode()

* Refactor string normalization to use case folding instead of lowercase conversion

* Refactor tests to use run_command_on_all_shards for setting DEFAULT_DIALECT

* BWC: Add function to convert string to single codepoint folded runes

* Rename rm_strdupcase() to rm_normalize()

* Add stemming test

* Refactor string normalization to use lowercase transformation instead of folding.
Modify FT.SUGGET flow to be BWC

* Refactor filtering functions to use a transformation callback for rune processing. Remove dead code.

* Update testDFAFilter to use strToSingleCodepointFoldedRunes for rune processing

* Refactor string normalization to use case folding (single codepoint) instead of lowercase transformation

* Revert "Refactor string normalization to use case folding (single codepoint) instead of lowercase transformation"

This reverts commit ebcf994.

* Add case sensitivity option to tag string processing functions

* Rename tag_strtofold to tag_strtolower for clarity and update related references to reflect lowercase transformation

* Simplify length checks in tag_strtolower and rm_normalize functions

* Add documentation after review

* WIP: Enhance unicode_tolower function to handle memory allocation for transformed strings

* WIP: Fixing unicode_tolower usage

* Fix tag_strtolower() to support memory realloc

* skip testToLowerSize() on cluster

* Fix rm_normalize and add more tests

* Fix memory leak

* Fix memory leak 2

* MOD-9609: Pass Private Data Context Back To UnblockClient API (#6237)

* Apparently private data had to be sent back when unblocking the client in order for the free function to get called

* code review comment

* solve memory leak

* MOD-9774: Stop Compiling Redis-With Coverage (#6231)

* stop compiling redis-server with coverage

* bring back coverage as requirement

* Add unit test for unicode_tolower

* Fix memory management in UnicodeToLowerTest and update expected results in multibyte character tests

* Add tests for unsupported Unicode characters in tolower conversion.

* MOD-9768: Add Rust abstractions for Buffer* (#6175)

* Add Rust abstractions for Buffer*

* Add tests

* Make Buffer fields pub

For consistency with BufferReader and BufferWriter. Also there is a very
good chance of us needing to use them directly in the near future.

* Expand the docs a bit more

* Add and improve upon safety comments

* fmt fix

* Add forgotten license headers

* Revert "Make Buffer fields pub"

This reverts commit 1243590.

* Address review comments

* Address review comments

* Better document safety of `Buffer` fields

* Apply suggestions from code review

Co-authored-by: Luca Palmieri <[email protected]>

* Remove an unneeded import

* Use reallocation in mock `Buffer_Grow`

* Satisfy MIRI about some pointer access

* Drop unneeded dep

* Remove unnecessary #includes

* Split FFI into a separate crate

* Add buffer.h API to ffi crate

* Buffer API now based on generated bindings for the C Buffer API

* CI: Set `CARGO_PROFILE_OPTIMISED_TEST_BUILD_OVERRIDE_DEBUG` env

Just acting on the advice from a panic:

```
To improve backtraces for build dependencies, set the CARGO_PROFILE_OPTIMISED_TEST_BUILD_OVERRIDE_DEBUG=true environment variable to enable debug information generation.
```

* Drop linking of internal libs in ffi crate

* Apply suggestions from code review

Co-authored-by: Luca Palmieri <[email protected]>

* Revert "CI: Set `CARGO_PROFILE_OPTIMISED_TEST_BUILD_OVERRIDE_DEBUG` env"

This reverts commit 9bd1d87.

* Drop a redundant import

* Add a few debug_asserts

* Bring back an accidentally deleted assertion in a test

* Fix a .gitignore file

* Make a safety assumption explicit

* Dont build `buffer` crate as static lib

* Don't build bindings for oxidized triemap code

* Make use of BufferWritter::buffer_mut

* Fix build on Alpine

---------

Co-authored-by: Luca Palmieri <[email protected]>

* refactor EmptyIterator (#6240)

* refactor EmptyIterator

* fix useless imports and doc

* update empty_iterator.c to add newline

* add gcc11:bullseye to platforms [MOD-9529] (#6198)

* add gcc11:bullseye to platforms

* add naming

* remove deps

* add nightly platforms option

* set merge queue to false

* fix flow

* fix flow

* chore: Improve granularity of build invalidation logic for FFI bindings (#6244)

* Add `tests/deps/RedisJSON` to `.gitignore` (#6249)

* Skip tests of unsupported unicode 9.0.0 chars

* Fix mem leak 3

* Test stopwords which require memory reallocation

* unicode_tolower update out_len always

* Refactor unicode_tolower function to receive a single  *inout_len argument

* Update documentation for tag_strtolower()

* Fixes after review

* tokenizeTagString: Rename origLen as len

* Test prefix/infix/suffix search with supported unicode chars

* Update test description

* Fix testToLowerConversionExactMatch

* testTextToLowerConversionSimilarMatch: remove unused doc_id in queries

* fix crash in test

* Fix test failure on focal and bullseye: avoid nested f-strings with curly brackets

* Fix test failure on mac: invalid escape sequence

* Use SSO in runesToStr and strToLowerRunes

* Add utf8_len arg to strToLowerRunes()

* Align and move strToLowerRunes comment to header

---------

Co-authored-by: kei-nan <[email protected]>
Co-authored-by: Zeeshan Ali Khan <[email protected]>
Co-authored-by: Luca Palmieri <[email protected]>
Co-authored-by: Joan Fontanals <[email protected]>
Co-authored-by: BenGoldberger <[email protected]>
Co-authored-by: oshadmi <[email protected]>
(cherry picked from commit dfe511e)
github-merge-queue bot pushed a commit that referenced this pull request Jun 17, 2025
MOD-8799: Support special utf8 (#5637)

* Normalize multi-byte char terms

* Fix rm_strdupcase_utf8

* Remove debug log

* Add Russian alphabet and diacritic tests

* Don't use DefaultNormalize_utf8() for chinese

* print available locales

* Check locale before using utf8 normalization

* Fix DefaultNormalize()

* Skip multibyte tests if 'en_US.UTF-8' locale is not available

* Test multibyte stopwords

* Revert changes in install_script.sh

* run sanitizer using ubuntu:latest

* Test_cn: fix language_field

* Remove unused strtolower function from misc.c and misc.h

* Fixes from code review

* Enhance multibyte character tests

* Validate queries using multi-byte stopwords

* Add MULTIBYTE_CHARS config param and install locales

* Use setlocale instead of querylocale for better compatibility

* revert changes in event-pull_request.yml and install French locale for debian/ubuntu

* Convert to lowercase using nunicode library

* nunicode_tolower() returns zero terminated string

* Refactor nunicode_tolower() to improve readability

* Refactor nunicode_tolower() to use destination buffer and improve memory management

* Remove null termination in nunicode_tolower()

* Improve documentation

* Support multi-byte chars for tags

* Support multi-byte chars for synonyms

* unicode_tolower(): avoid duplicating encoded input

* Increase SSO_MAX_LENGTH

* Update test to  use FT.DEBUG DUMP_TAGIDX instead of FT.TAGVALS

* Keep previous test_cn:testSynonym and test_cn:testMixedHighlight

* Update CMake configuration for consistent multi-byte char sorting

* Test JSON index

* Fix JSON test and remove unneeded tolower()

* Fix query_EvalSingleTagNode()

* Refactor string normalization to use case folding instead of lowercase conversion

* Refactor tests to use run_command_on_all_shards for setting DEFAULT_DIALECT

* BWC: Add function to convert string to single codepoint folded runes

* Rename rm_strdupcase() to rm_normalize()

* Add stemming test

* Refactor string normalization to use lowercase transformation instead of folding.
Modify FT.SUGGET flow to be BWC

* Refactor filtering functions to use a transformation callback for rune processing. Remove dead code.

* Update testDFAFilter to use strToSingleCodepointFoldedRunes for rune processing

* Refactor string normalization to use case folding (single codepoint) instead of lowercase transformation

* Revert "Refactor string normalization to use case folding (single codepoint) instead of lowercase transformation"

This reverts commit ebcf994.

* Add case sensitivity option to tag string processing functions

* Rename tag_strtofold to tag_strtolower for clarity and update related references to reflect lowercase transformation

* Simplify length checks in tag_strtolower and rm_normalize functions

* Add documentation after review

* WIP: Enhance unicode_tolower function to handle memory allocation for transformed strings

* WIP: Fixing unicode_tolower usage

* Fix tag_strtolower() to support memory realloc

* skip testToLowerSize() on cluster

* Fix rm_normalize and add more tests

* Fix memory leak

* Fix memory leak 2

* MOD-9609: Pass Private Data Context Back To UnblockClient API (#6237)

* Apparently private data had to be sent back when unblocking the client in order for the free function to get called

* code review comment

* solve memory leak

* MOD-9774: Stop Compiling Redis-With Coverage (#6231)

* stop compiling redis-server with coverage

* bring back coverage as requirement

* Add unit test for unicode_tolower

* Fix memory management in UnicodeToLowerTest and update expected results in multibyte character tests

* Add tests for unsupported Unicode characters in tolower conversion.

* MOD-9768: Add Rust abstractions for Buffer* (#6175)

* Add Rust abstractions for Buffer*

* Add tests

* Make Buffer fields pub

For consistency with BufferReader and BufferWriter. Also there is a very
good chance of us needing to use them directly in the near future.

* Expand the docs a bit more

* Add and improve upon safety comments

* fmt fix

* Add forgotten license headers

* Revert "Make Buffer fields pub"

This reverts commit 1243590.

* Address review comments

* Address review comments

* Better document safety of `Buffer` fields

* Apply suggestions from code review



* Remove an unneeded import

* Use reallocation in mock `Buffer_Grow`

* Satisfy MIRI about some pointer access

* Drop unneeded dep

* Remove unnecessary #includes

* Split FFI into a separate crate

* Add buffer.h API to ffi crate

* Buffer API now based on generated bindings for the C Buffer API

* CI: Set `CARGO_PROFILE_OPTIMISED_TEST_BUILD_OVERRIDE_DEBUG` env

Just acting on the advice from a panic:

```
To improve backtraces for build dependencies, set the CARGO_PROFILE_OPTIMISED_TEST_BUILD_OVERRIDE_DEBUG=true environment variable to enable debug information generation.
```

* Drop linking of internal libs in ffi crate

* Apply suggestions from code review



* Revert "CI: Set `CARGO_PROFILE_OPTIMISED_TEST_BUILD_OVERRIDE_DEBUG` env"

This reverts commit 9bd1d87.

* Drop a redundant import

* Add a few debug_asserts

* Bring back an accidentally deleted assertion in a test

* Fix a .gitignore file

* Make a safety assumption explicit

* Dont build `buffer` crate as static lib

* Don't build bindings for oxidized triemap code

* Make use of BufferWritter::buffer_mut

* Fix build on Alpine

---------



* refactor EmptyIterator (#6240)

* refactor EmptyIterator

* fix useless imports and doc

* update empty_iterator.c to add newline

* add gcc11:bullseye to platforms [MOD-9529] (#6198)

* add gcc11:bullseye to platforms

* add naming

* remove deps

* add nightly platforms option

* set merge queue to false

* fix flow

* fix flow

* chore: Improve granularity of build invalidation logic for FFI bindings (#6244)

* Add `tests/deps/RedisJSON` to `.gitignore` (#6249)

* Skip tests of unsupported unicode 9.0.0 chars

* Fix mem leak 3

* Test stopwords which require memory reallocation

* unicode_tolower update out_len always

* Refactor unicode_tolower function to receive a single  *inout_len argument

* Update documentation for tag_strtolower()

* Fixes after review

* tokenizeTagString: Rename origLen as len

* Test prefix/infix/suffix search with supported unicode chars

* Update test description

* Fix testToLowerConversionExactMatch

* testTextToLowerConversionSimilarMatch: remove unused doc_id in queries

* fix crash in test

* Fix test failure on focal and bullseye: avoid nested f-strings with curly brackets

* Fix test failure on mac: invalid escape sequence

* Use SSO in runesToStr and strToLowerRunes

* Add utf8_len arg to strToLowerRunes()

* Align and move strToLowerRunes comment to header

---------







(cherry picked from commit dfe511e)

Co-authored-by: nafraf <[email protected]>
Co-authored-by: kei-nan <[email protected]>
Co-authored-by: Zeeshan Ali Khan <[email protected]>
Co-authored-by: Luca Palmieri <[email protected]>
Co-authored-by: Joan Fontanals <[email protected]>
Co-authored-by: BenGoldberger <[email protected]>
Co-authored-by: oshadmi <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants