test: add count-min C++ serialization compatibility tests#92
Conversation
This comment was marked as outdated.
This comment was marked as outdated.
|
Failed on Windows. Perhaps related to line separator? |
|
Looks like when generating the cpp snapshot, it prints CRLF on windows. Should work with their people like @proost |
|
WRT Windows and CRLF issues. I noticed that there is no .gitattributes file in ds-cpp. I ran into similar issues recently when refactoring ds-memory. It turns out all the major modern OSs can correctly process text files with just LF "\n" line endings. So structuring the .gitattributes file to force LF endings for all text type files fixed the problem and it works for Ubuntu, MacOS, and Windows. See this .gitattributes. |
|
@PsiACE the corresponding cpp patch has been merged. You may rebase this patch and pin the cpp checkout to the latest commit and retry. |
PsiACE
force-pushed
the
countmin-xlang-serde-tests
branch
from
cb2c7f2 to
837bda2
Compare
|
Rebased this branch onto current |
PsiACE
force-pushed
the
countmin-xlang-serde-tests
branch
from
837bda2 to
bf64c87
Compare
PsiACE
changed the title
PsiACE
force-pushed
the
countmin-xlang-serde-tests
branch
from
bf64c87 to
76ffa56
Compare
|
Seems C++ snapshot generation issue, not gitattribute. Let me prepare a fix. |
|
|
You may test with checkout to this patch's commit so we use it as an e2e test. |
| @@ -142,6 +146,8 @@ def generate_cpp_files(workspace_dir, project_root): | |||
| repo_url, | |||
| str(temp_dir) | |||
| ]) | |||
| run_command(["git", "fetch", "--depth", "1", "origin", fetch_ref], cwd=temp_dir) | |||
| run_command(["git", "checkout", "--detach", commit], cwd=temp_dir) | |||
There was a problem hiding this comment.
some temp code here; needs to be cleaned once PR505 merged into c++ impl
tisonkun
left a comment
tisonkun
left a comment
There was a problem hiding this comment.
LGTM - merging now.
We can drop python scripts trick once datasketches-cpp updated.
3 participants
No description provided.