Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add faulty injection in writer fuzzer #11375

Closed
wants to merge 1 commit into from
Closed

Add faulty injection in writer fuzzer #11375

wants to merge 1 commit into from

Conversation

kewang1024
Copy link
Contributor

No description provided.

@facebook-github-bot facebook-github-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 Oct 29, 2024
@netlify Netlify
Copy link

netlify bot commented Oct 29, 2024 •
edited
Loading

✅ Deploy Preview for meta-velox canceled.

Name Link
🔨 Latest commit 1df9a3a
🔍 Latest deploy log https://app.netlify.com/sites/meta-velox/deploys/672fa36b1dc7d90008b6e3de

xiaoxmeng
xiaoxmeng reviewed Oct 31, 2024
View reviewed changes
velox/exec/fuzzer/WriterFuzzer.cpp Outdated Show resolved Hide resolved
velox/exec/fuzzer/WriterFuzzer.cpp Outdated Show resolved Hide resolved
@kewang1024 kewang1024 force-pushed the faulty branch 5 times, most recently from 6bee697 to d487c5f Compare November 2, 2024 08:56
@kewang1024 kewang1024 marked this pull request as ready for review November 2, 2024 08:56
@facebook-github-bot
Copy link
Contributor

@kewang1024 has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

@facebook-github-bot
Copy link
Contributor

@kewang1024 has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

@kewang1024 kewang1024 force-pushed the faulty branch 2 times, most recently from d5ba223 to 400ec12 Compare November 4, 2024 00:13
@facebook-github-bot
Copy link
Contributor

@kewang1024 has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

@kewang1024 kewang1024 force-pushed the faulty branch 2 times, most recently from e69f429 to 35a79e0 Compare November 4, 2024 05:15
@facebook-github-bot
Copy link
Contributor

@kewang1024 has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

xiaoxmeng
xiaoxmeng reviewed Nov 5, 2024
View reviewed changes
velox/dwio/common/FileSink.cpp Outdated Show resolved Hide resolved
velox/dwio/common/FileSink.h Outdated Show resolved Hide resolved
velox/dwio/common/tests/FaultyFileSink.h Outdated Show resolved Hide resolved
velox/dwio/common/tests/FaultyFileSink.h Outdated Show resolved Hide resolved
velox/exec/fuzzer/WriterFuzzer.cpp Outdated Show resolved Hide resolved
@kewang1024 kewang1024 force-pushed the faulty branch 2 times, most recently from 734200f to a31d76d Compare November 5, 2024 19:36
xiaoxmeng
xiaoxmeng reviewed Nov 5, 2024
View reviewed changes
velox/dwio/common/FileSink.h Outdated Show resolved Hide resolved
velox/dwio/common/tests/FaultyFileSink.h Outdated Show resolved Hide resolved
@facebook-github-bot
Copy link
Contributor

@kewang1024 has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

@kewang1024 kewang1024 force-pushed the faulty branch 2 times, most recently from 58fc028 to 4f24a84 Compare November 6, 2024 08:27
xiaoxmeng
xiaoxmeng reviewed Nov 6, 2024
View reviewed changes
Copy link
Contributor

@xiaoxmeng xiaoxmeng left a comment

Choose a reason for hiding this comment

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

@kewang1024 LGTM % minors. Thanks!

velox/exec/tests/utils/TempDirectoryPath.h Show resolved Hide resolved
velox/exec/fuzzer/WriterFuzzer.cpp Outdated Show resolved Hide resolved
velox/exec/fuzzer/WriterFuzzer.cpp Outdated Show resolved Hide resolved
velox/exec/fuzzer/WriterFuzzer.cpp Outdated Show resolved Hide resolved
velox/exec/fuzzer/WriterFuzzer.cpp Show resolved Hide resolved
velox/exec/fuzzer/WriterFuzzer.cpp Outdated Show resolved Hide resolved
@facebook-github-bot
Copy link
Contributor

@kewang1024 has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

xiaoxmeng
xiaoxmeng approved these changes Nov 6, 2024
View reviewed changes
Copy link
Contributor

@xiaoxmeng xiaoxmeng left a comment

Choose a reason for hiding this comment

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

@kewang1024 LGTM. Thanks for the update!

velox/exec/fuzzer/WriterFuzzer.cpp Outdated Show resolved Hide resolved
kewang1024 added a commit to kewang1024/velox that referenced this pull request Nov 6, 2024
@kewang1024 @facebook-github-bot
Add faulty injection in writer fuzzer (facebookincubator#11375)
45abe71 
Summary: Pull Request resolved: facebookincubator#11375

Differential Revision: D65380360

Pulled By: kewang1024
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D65380360

kewang1024 added a commit to kewang1024/velox that referenced this pull request Nov 6, 2024
@kewang1024 @facebook-github-bot
Add faulty injection in writer fuzzer (facebookincubator#11375)
fb9222f 
Summary: Pull Request resolved: facebookincubator#11375

Reviewed By: xiaoxmeng

Differential Revision: D65380360

Pulled By: kewang1024
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D65380360

assignUser
assignUser reviewed Nov 8, 2024
View reviewed changes
velox/dwio/common/tests/CMakeLists.txt
Comment on lines +16 to +17
velox_add_library(velox_dwio_faulty_file_sink FaultyFileSink.cpp)
velox_link_libraries(velox_dwio_faulty_file_sink velox_file_test_utils
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is a test utility right? In that case it should maybe live in utils but it should def use the base cmake functions add_library and target_link_libraries to create a standalone lib, that way when we can install velox you can build with test stuff but the actual library is still 'clean'.

@kewang1024 @facebook-github-bot
Add faulty injection in writer fuzzer (facebookincubator#11375)
1df9a3a 
Summary: Pull Request resolved: facebookincubator#11375

Reviewed By: xiaoxmeng

Differential Revision: D65380360

Pulled By: kewang1024
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D65380360

@facebook-github-bot
Copy link
Contributor

@kewang1024 merged this pull request in ec82503.

@conbench-facebook conbench-facebook
Copy link

Conbench analyzed the 1 benchmark run on commit ec825034.

There were no benchmark performance regressions. 🎉

The full Conbench report has more details.

athmaja-n pushed a commit to athmaja-n/velox that referenced this pull request Jan 10, 2025
@kewang1024 @athmaja-n
Add faulty injection in writer fuzzer (facebookincubator#11375)
f70a1a5 
Summary: Pull Request resolved: facebookincubator#11375

Reviewed By: xiaoxmeng

Differential Revision: D65380360

Pulled By: kewang1024

fbshipit-source-id: 31c31aa73a8a045d26d1157be731e54754f92dd9
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@assignUser assignUser assignUser left review comments

@xiaoxmeng xiaoxmeng xiaoxmeng approved these changes

@majetideepak majetideepak Awaiting requested review from majetideepak majetideepak is a code owner

Assignees
No one assigned
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. fb-exported Merged
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@kewang1024 @facebook-github-bot @assignUser @xiaoxmeng