Skip to content

fix(ci): Re-enable velox_table_evolution_fuzzer_test in CPU builds#309

Open
simoneves wants to merge 1 commit intomainfrom
simoneves/re-enable_velox_table_evolution_fuzzer_test
Open

fix(ci): Re-enable velox_table_evolution_fuzzer_test in CPU builds#309
simoneves wants to merge 1 commit intomainfrom
simoneves/re-enable_velox_table_evolution_fuzzer_test

Conversation

@simoneves
Copy link
Copy Markdown
Contributor

Once facebookincubator/velox#17106 lands, this test can once more be safely run in CI.

Copy link
Copy Markdown
Contributor

@mattgara mattgara left a comment

Choose a reason for hiding this comment

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

LGTM, is there a passing example run with this enabled?

@simoneves
Copy link
Copy Markdown
Contributor Author

We can test this part officially once the fix lands.

@Avinash-Raj
Copy link
Copy Markdown
Contributor

But this script test_velox.sh is no longer being called from a velox-test.yml CI workflow.

@simoneves
Copy link
Copy Markdown
Contributor Author

But this script test_velox.sh is no longer being called from a velox-test.yml CI workflow.

Oops! OK... what runs tests now then? Does it need an equivalent change. Would look but on my phone and too lazy to fire up the computer...

@bdice
Copy link
Copy Markdown
Contributor

bdice commented Apr 12, 2026

The CPU test command is here:

docker run --rm "${IMAGE}" bash -c \
"cd /opt/velox-build/release && ctest -j 16 --label-exclude cuda_driver --output-on-failure --no-tests=error"

If we are supposed to use a common/shared test script, that's fine with me as long as we can call it through Docker with a custom image like the current GitHub Actions setup.

@simoneves
Copy link
Copy Markdown
Contributor Author

If we are supposed to use a common/shared test script, that's fine with me as long as we can call it through Docker with a custom image like the current GitHub Actions setup.

If the new YAML implementation does the job, I don't see a need to change it. Presumably this is how we were reminded that the table_evolution_fuzzer test was problematic, as the old script didn't run it. Also presumably there are no issues with the other three tests the old script didn't run either.

If that's now the definitive method, should we remove the test_velox.sh script completely, or leave it for posterity?

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.

4 participants