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

Generated test files should be written to /tmp and not the test directory #104

Open
2 tasks
shannonwells opened this issue Nov 21, 2023 · 0 comments
Open
2 tasks
Labels
chore good first issue Good for newcomers

Comments

@shannonwells
Copy link
Collaborator

shannonwells commented Nov 21, 2023

Problem

This behavior is from the original version that this repo forked from. It writes test files to the test directory and never cleans them up. There are test files already in test/test-files, which are also used by some tests. This can be confusing for debugging and actually did confuse me when trying to debug some test failures.

Secondly, the test named "reads parquet files via http" in test/reader.js depends upon the file generated by bloomFilterIntegration.ts and that is poor test practice.

Solution

  • Write generated test files to /tmp where they will be cleaned up automatically by the system. This is the kind of thing /tmp is for, and IMO this is the best choice rather than having to create and maintain cleanup test code.
  • Replace hard-coded strings used throughout tests for file names and locations in test with constants and use that instead.

Test code that reads generated files will have to be correctly distinguished from test code that opens test/test-files, and point to the files in /tmp.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
chore good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

2 participants