search

hildogjr / KiCost

Build cost spreadsheet for a KiCad project.
MIT License
496 stars 97 forks source link

Include the prices in the tests #463

Closed set-soft closed 3 years ago

set-soft commented 3 years ago

This patch allows testing the most important KiCost feature: getting the costs. To achieve it I'm adding a fake web server that can pretend to be KitSpace's server, but using recorded responses. In this way we can reproduce the transactions over and over. The patch also allows to record the transactions and make the currency exchange rates reproducible.

set-soft commented 3 years ago

Hi @hildogjr ! This PR is in progress, but I want you to take a look at what I'm doing and tell me your opinion. The main idea is to test KiCost a little bit better. I think the current regression tests are unacceptable. They are too naive and only a minimal part of the functionality is really tested. This patch tries to cover more functionality.

set-soft commented 3 years ago

At this point the patch seems to be usable. We just need to extend it to all the tests. BTW: I don't understand the purpose of each test. It looks like a random collection of fuzzy tests. The tests should be named in a way that we know what's their purpose. Like I did for tests like multipart and variant. With the current names if a test fails we don't know what it means.

set-soft commented 3 years ago

Question: query_part_info reeceives distributors as argument, but this value is completly ignored. Even more confusing: FIELDS_CAT (and DISTRIBUTORS) are created from distributor_dict. Then the code computes distributors to remove the distributors not supported by KitSpace API. But the query creation is done with the first two. Is quite confusing.

set-soft commented 3 years ago

Hi @hildogjr ! Now the patch seems to be fully functional. All tests can be run with price. I collected 3.2 MiB of queries covering all the tests. In the process I made the code much more reproducible. Note that 32bbfcab174ed4fcca4b9a18219c40f8adab782b fixes a problem with subparts. I made a lot of changes to the KitSpace API, most of them to make the output consistent, but some of them to make the code more clear. The fake webserver is working quite well, so we can test reproducible values and really fast.

hildogjr commented 3 years ago

Question: query_part_info reeceives distributors as argument, but this value is completly ignored.

KiCost have the capability of "create local virtual distributor" ("hard coding" the price into the an specific filed of the part into the schematic, by using DIST_NAME:pricing = 1:USDxx.xx ; 10:USDxxx, which also allow the alpha3 currency name). This feature use the file dist_local_tempalte.py and the function query_part_info(foo) uses the query_part_info name argument as this virtual distributor name.

This is an attempt to generate a general distributors template https://github.com/xesscorp/KiCost/issues/386: dist_local_template.py and api_partinfo_kitspace.py are in use, api_octopart.py may work but need code change at line 95 of kicost.py to call (in the road map of KiCost I would like to make the new APIs/scrap/local module addition more transparent to programmers, see line 37 of distributors/__init__.py). api_octopart.py was kept from older version to users that have big server request quantity (it was the first API written, before Kitspace).

Even more confusing: FIELDS_CAT (and DISTRIBUTORS) are created from distributor_dict. Then the code computes distributors to remove the distributors not supported by KitSpace API. But the query creation is done with the first two. Is quite confusing.

If I remember correctly this was just a "band aid" fix, removing it was necessary to not create errors on spreadsheet creation.

@set-soft thanks by the wonderful improvements on test and code revisions, I am checking your work now. @devbisme / @xesscorp (KiCost's creator) may consider grant so maintenance privileges of the repository to @set-soft.

xesscorp commented 3 years ago

I have invited @set-soft to be a collaborator. Thanks for making these improvements, and thanks to @hildogjr for continuing to drive the project forward.