-
Notifications
You must be signed in to change notification settings - Fork 1k
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.
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
runtime-rs: fix the bug of func count_files #9830
base: main
Are you sure you want to change the base?
Conversation
ffd89d4
to
486d4b9
Compare
/test |
@gaohuatao-1 Thanks for bringing this to runtime-rs. Could you mind adding some unit tests to the method? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thx @gaohuatao-1 It looks good for me but lacks the UT of such general function.
486d4b9
to
16ab017
Compare
/test |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lgtm, thanks!
When the total number of files observed is greater than limit, return -1 directly. runtime has fixed this bug, it should b ported to runtime-rs. Fixes:kata-containers#9829 Signed-off-by: gaohuatao <gaohuatao@bytedance.com>
16ab017
to
4cb4e44
Compare
@justxuewei @Apokleos I have added a ut to the method. PTAL again, thanks. |
/test |
@Apokleos Hi, PTAL, Thanks. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thx @gaohuatao-1 Compared with CountFiles in kata runtime with golang
, this commit looks good to me.
But I try to set the limit < 0
(limit declared as i32), the UT fails. I think the argument limit: i32
in count_files() shoud be change as limit: u32
, maybe it's another bug, what do you think ?
|
When the total number of files observed is greater than limit, return -1 directly. runtime has fixed this bug, it should b ported to runtime-rs.
Fixes:#9829