Skip to content

Fix distributed CSV comparison test#12

Merged
JulienLoiseau merged 1 commit into
lanl:mainfrom
yrasool:fix-distributed-csv-comparison
Jul 8, 2026
Merged

Fix distributed CSV comparison test#12
JulienLoiseau merged 1 commit into
lanl:mainfrom
yrasool:fix-distributed-csv-comparison

Conversation

@yrasool

@yrasool yrasool commented Jun 22, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Compare distributed test CSV output using comma delimiters.
  • Remove noisy directory listings from the test script.
  • Show a unified diff when the single-rank and distributed outputs differ.
  • Use tolerant cleanup for generated CSV files.

Testing

  • bash -n tools/test_distributed.sh
  • git diff --check
  • Tried configuring the full HARD test build in WSL with cmake -S . -B build -G Ninja -DENABLE_UNIT_TESTS=ON -DENABLE_VERIFICATION=ON. Configure stopped because FleCSI is not installed in my local WSL environment.

@JulienLoiseau

Copy link
Copy Markdown
Collaborator

We have huge changes of the code coming soon with a new Spack and a new structure for the different applications. I will wait for these changes to be pushed before merging this MR. I hope to push before the end of the month. Thank you!

@JulienLoiseau

Copy link
Copy Markdown
Collaborator

Sorry for the delay! The new version of the code is finally out.

@yrasool

yrasool commented Jul 6, 2026

Copy link
Copy Markdown
Contributor Author

Sorry for the delay! The new version of the code is finally out.

Thankyou for your message . I will review the new code and make changes accordingly.

@yrasool yrasool force-pushed the fix-distributed-csv-comparison branch from c408cc9 to f5f4d98 Compare July 7, 2026 23:03
@yrasool

yrasool commented Jul 8, 2026

Copy link
Copy Markdown
Contributor Author

Hello, I saw the new code changes. However, distributed regression test in the new codebase was still reading those .csv files as tab-separated text.

@JulienLoiseau

Copy link
Copy Markdown
Collaborator

Thank you for the fix, I tested it on our CI internally and this causes the test to fail (as it should!) I will merge this here and proceed to push a fix to the github asap for the failing test.

@yrasool

yrasool commented Jul 8, 2026

Copy link
Copy Markdown
Contributor Author

Appreciate it. Will love to learn more and contribute more.

@JulienLoiseau JulienLoiseau merged commit dcf5a85 into lanl:main Jul 8, 2026
1 check passed
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