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.

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

test: add fs_huge_copyfile test #1553

Closed
wants to merge 3 commits into from

Conversation

santigimeno
Copy link
Member

It checks copying files larger than 0x7ffff000 works as expected.
Refactor touch_file() function on test-fs-copyfile.c so it's faster.
Disabled on some platforms for different reasons:

  • OS X and Windows as sometimes the test would timeout even after
    having expanded the allowed time. Also, we're specifically testing
    partial sendfile() calls which don't apply to these platforms.
  • AIX as apparently by default doesn't support files larger than 2GB.

Refs: #1494

@santigimeno
Copy link
Member Author

@cjihrig
Copy link
Contributor

cjihrig commented Sep 20, 2017

OS X and Windows as sometimes the test would timeout even after
having expanded the allowed time

It's interesting because these are the only two platforms that use system specific APIs for file copying. Are you saying it's not possible to copy files that large using the existing implementation?

@santigimeno
Copy link
Member Author

Are you saying it's not possible to copy files that large using the existing implementation?

Not really
On OS X the test was flaky in the CI: sometimes succeeded and sometimes not.
On windows, on some of the versions ( I can't remember right now) always timed out, on others it worked just fine.

test/test-list.h Outdated
@@ -275,6 +275,9 @@ TEST_DECLARE (fs_fstat)
TEST_DECLARE (fs_access)
TEST_DECLARE (fs_chmod)
TEST_DECLARE (fs_copyfile)
#if !defined(_WIN32) && !defined(_AIX) && !defined(__APPLE__)
Copy link
Member

Choose a reason for hiding this comment

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

Suggestion: define unconditionally and move the #if guard inside the function in test/test-fs-copyfile.c, doing a RETURN_SKIP on unsupported platforms?

@santigimeno santigimeno force-pushed the copyfile_test branch 2 times, most recently from 1ae1997 to 1408987 Compare October 5, 2017 21:57
@santigimeno
Copy link
Member Author

Suggestion incorporated. CI: https://ci.nodejs.org/view/libuv/job/libuv-test-commit/522/

It checks copying files larger than 0x7ffff000 works as expected.
Refactor `touch_file()` function on `test-fs-copyfile.c` so it's faster.
Disabled on some platforms for different reasons:
- `OS X` and `Windows` as sometimes the test would timeout even after
  having expanded the allowed time. Also, we're specifically testing
  partial `sendfile()` calls which don't apply to these platforms.
- `AIX` as apparently by default doesn't support files larger than 2GB.

PR-URL: libuv#1596
Refs: libuv#1494
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
@cjihrig
Copy link
Contributor

cjihrig commented Apr 2, 2018

@santigimeno do you want to keep this open, or did you decide that some CI systems might make this not worth it?

@santigimeno
Copy link
Member Author

Yeah, it was terribly flaky. Let's close it for the moment.

@santigimeno santigimeno closed this Apr 2, 2018
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.

None yet

3 participants