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

tests: inconsistent CSV format between test server.csv and main server.csv #327

Open
Shillaker opened this issue Oct 4, 2024 · 3 comments
Labels
help wanted Extra attention is needed technical

Comments

@Shillaker
Copy link
Contributor

Bug description

The server.csv file used in the tests has extra columns vs. the main server.csv file file (notably RAM.density and SSD.density).

The second row in the tests file has the wrong number of columns and prevents GitHub from rendering it nicely as a CSV.

To Reproduce

N/A

Expected behavior

Both files have the same columns, and all the rows have the correct number of entries.

JSON OUTPUT

N/A

Additional context

N/A

@Shillaker Shillaker added the bug Something isn't working label Oct 4, 2024
@Shillaker
Copy link
Contributor Author

Shillaker commented Oct 4, 2024

What is the advantage of having tests/data vs. just using boaviztapi/data directly in the tests? It looks like a lot of the data is copy-pasted, or has become out of sync.

@da-ekchajzer
Copy link
Collaborator

Thanks for bringing this up, it's one of the things on my mind right now.

Since the data used in the API is supposed to change regularly, it would be problematic to base tests on a constantly changing data set.

For example, we sometimes use the average die for a certain type of CPU. If we added more CPUs, the average would change and the tests would fail. So the goal was to keep a directory that was unlikely to change (or change very little) to avoid having tests fail when we add data without touching the code.

The problem you're pointing out is that files can move and become completely out of sync.

What would you suggest?

@Shillaker
Copy link
Contributor Author

I see what you mean about changing data and breaking tests, this would be painful. However, there are still some instances where it would be good to change the tests when the data changes, e.g. if someone adds a new column to the CSVs.

If we want to keep the test CSVs we could do the following:

  • Reduce the test CSVs to the absolute minimum number of lines. AFAICT the tests only use a small amount of the data that's there. Smaller files are easier to maintain.
  • Add a new test that checks the columns in the test files are the same as the columns in the main data files. This test will only fail when a new column is added to the main file, then the author will need to update the test CSV files. This keeps them in sync.

Another alternative is to programmatically generate the test data on the fly as test fixtures, rather than put it in a CSV. This would avoid needing to maintain a single CSV file for all the tests, and instead, each test would generate its own data. In the past I've used FactoryBoy for stuff like this, which is great for generating dummy objects on the fly with "sensible" defaults.

@da-ekchajzer da-ekchajzer added technical help wanted Extra attention is needed and removed bug Something isn't working labels Dec 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Extra attention is needed technical
Projects
None yet
Development

No branches or pull requests

2 participants