Skip to content

ARROW-8217: [R] Unskip previously failing test on Win32 in test-dataset.R from ARROW-7979#6799

Closed
wesm wants to merge 2 commits intoapache:masterfrom
wesm:unskip-failing-r-test
Closed

ARROW-8217: [R] Unskip previously failing test on Win32 in test-dataset.R from ARROW-7979#6799
wesm wants to merge 2 commits intoapache:masterfrom
wesm:unskip-failing-r-test

Conversation

@wesm
Copy link
Copy Markdown
Member

@wesm wesm commented Apr 1, 2020

Neal found that the test passed in a debug build, let's see if the test still fails on master

@github-actions
Copy link
Copy Markdown

github-actions bot commented Apr 1, 2020

@wesm
Copy link
Copy Markdown
Member Author

wesm commented Apr 1, 2020

Good grief. I'll start bisecting by hand on my fork to see if I can determine which patch fixed this. In the meantime we can merge this, I'm going to remove the commented-out code

@wesm wesm changed the title ARROW-8217: [R] Try unskipping failing test on Win32 in test-dataset.R ARROW-8217: [R] Unskip previously failing test on Win32 in test-dataset.R from ARROW-7979 Apr 1, 2020
@wesm
Copy link
Copy Markdown
Member Author

wesm commented Apr 1, 2020

OK, @nealrichardson @fsaintjacques I think the mystery is solved. This appears to have been fixed by b9b9ece (see the RTools runs at https://github.com/wesm/arrow/branches/all?query=bisect -- ignore the red X's and click inside to see whether the RTools run succeeded or failed). So apparently there was some memory corruption happening that only provoked a segfault in the 32-bit build. Not sure

Good question is whether valgrind would have detected this on Linux builds of the R library or similar.

@nealrichardson
Copy link
Copy Markdown
Member

😅 all's well that ends well? Merge this?

@wesm
Copy link
Copy Markdown
Member Author

wesm commented Apr 1, 2020

Well, what's weird is that the failing test should not have been affected by the bug that was fixed in b9b9ece, so there is still some mystery here of understanding where the bad memory access or undefined behavior originated. If it comes up again we can revert to this discussion and produce a build prior to b9b9ece so that we can reproduce the segfault again and get a gdb backtrace

@wesm
Copy link
Copy Markdown
Member Author

wesm commented Apr 1, 2020

+1, anyway, merging this

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.

2 participants