search

calgo-lab / green-db

The monorepo that powers the GreenDB.
https://calgo-lab.github.io/green-db/
22 stars 2 forks source link

Add tests for extraction #37

Closed BigDatalex closed 2 years ago

BigDatalex commented 2 years ago

Tests for otto and zalando from the old green-db-integration repo have been added and adjusted according to the new product class and other minor changes.

se-jaeger commented 2 years ago

However, the rest looks good! Great to have tests now!

se-jaeger commented 2 years ago

I commented some minor things. And some more in the following:

  1. I would prefer rename the test files. For example: t-shirt_test.py and damen-pullover_test.py this makes it clear which HTML it uses. Since they are in the subfolder zalando/Otto its clear where they belong to.
  2. should we use another subfolder for the HTML file(s)?
  3. Could you implement a workflow that executes all tests when creating an PR. Examples are in .github/workflows

What do you think?

Oh and I forgot to mention: Maybe just utils.py instead of test_utils.py?

BigDatalex commented 2 years ago

Thanks for the review. Comments on your comments:

  1. Renaming the test files makes totally sence.
  2. Currently we only have one test file for each merchant, so I think additional folders wouldn't improve the file/directory overview. But if we want to add more test_files and html files in the future, we should add another folder for the test-data.
  3. I will check it out.
se-jaeger commented 2 years ago

2. Currently we only have one test file for each merchant, so I think additional folders wouldn't improve the file/directory overview. But if we want to add more test_files and html files in the future, we should add another folder for the test-data.

We definitely should ;)

BigDatalex commented 2 years ago
  1. Currently we only have one test file for each merchant, so I think additional folders wouldn't improve the file/directory overview. But if we want to add more test_files and html files in the future, we should add another folder for the test-data.

We definitely should ;)

Ok, so should I add then a subfolder "data" for each merchant?