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

taxonomy: added changes from laralem #8312

Merged
merged 3 commits into from
Apr 14, 2023
Merged

Conversation

benbenben2
Copy link
Collaborator

trying to re-do the PR #5160 from @laralem... after 2 years!

Due to the evolution of the file for the last 2 years, the following changes from the previous PR could not be added:

pt:gordura insaturada
pt:Gordura insaturada, Ácidos gordos insaturados, Lípidos insaturados
...

pt-pt:Colesterol

...
pt:Vitaminas, Vitamina
...
pt:Sais minerais, Minerais, Mineral, Matéria mineral, Sal mineral
...
pt:Cloro, Cl2, Cℓ, Cl-
...
pt:Sulfato, SO42-, SO4
...
pt:Frutas - vegetais e frutos secos (estimativa da lista de ingredientes)
...
pt:Grau de nutrição
...
pt:Clorofilas, Clorofila
...
pt:Resíduo seco a 180 °C, Resíduos secos a 180 °C
...
pt:EBU, European Bitterness Unit

@benbenben2 benbenben2 self-assigned this Apr 11, 2023
@github-actions github-actions bot added the 🧬 Taxonomies https://wiki.openfoodfacts.org/Global_taxonomies label Apr 11, 2023
@codecov-commenter
Copy link

Codecov Report

Merging #8312 (3338840) into main (98625ff) will increase coverage by 0.00%.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main    #8312   +/-   ##
=======================================
  Coverage   47.06%   47.07%           
=======================================
  Files         109      109           
  Lines       20991    20991           
  Branches     4739     4739           
=======================================
+ Hits         9880     9881    +1     
  Misses       9942     9942           
+ Partials     1169     1168    -1     

see 1 file with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@benbenben2
Copy link
Collaborator Author

2023-04-11T20:23:50.6416570Z tests/unit/packaging.t ........................ ok
2023-04-11T20:24:08.7186155Z tests/unit/parse_origins_from_text.t .......... ok
2023-04-11T20:24:23.3793117Z tests/unit/process_product_edit_rules.t ....... ok
2023-04-11T20:24:30.2911532Z Initializing Minion backend configured in lib/ProductOpener/Config2.pm
2023-04-11T20:26:03.5916641Z
2023-04-11T19:41:48.5633991Z # Failed test at /opt/product-opener/lib/ProductOpener/Test.pm line 329.
2023-04-11T19:41:48.5634480Z #
2023-04-11T19:41:48.5643250Z # Structures begin differing at:
2023-04-11T19:41:48.5643964Z # $got->{nitrate-ug-par-portion} = HASH(0x55a852c24de8)
2023-04-11T19:41:48.5644935Z # $expected->{nitrate-ug-par-portion} = Does not exist

2023-04-11T19:42:52.9679569Z # Failed test at /opt/product-opener/lib/ProductOpener/Test.pm line 329.
2023-04-11T19:42:52.9679943Z #
2023-04-11T19:42:52.9680065Z # Structures begin differing at:
2023-04-11T19:42:52.9680305Z # $got->{nitrato-for-100g-calories} = HASH(0x55a86bdbe6c8)
2023-04-11T19:42:52.9680548Z # $expected->{nitrato-for-100g-calories} = Does not exist

It was related to:

de:Nitrate
es:Nitrato
fr:Nitrate
pt:Nitrato, NO3
unit:en:
wikidata:en:Q182168
wikipedia:en:https://en.wikipedia.org/wiki/Nitrate

I understand that it is related to the subroutine compare_to_expected_results in lib/ProductOpener/Test.pm
That one is called in tests/unit/producers.t with three files column_names_$l.json ($l = en, fr, es)
For en, the file contain some entries with "nitrate" (2190 times in the file!).
For fr and for es, the files do not contain entries with "nitrate" or "nitrato". This explain why we get the error because we expected nothing and we got a result as the fr:Nitrate and es:Nitrato are in taxonomy... Do we have to add nitrate and nitrato in files... 2190 times???
Is there a way to generate the files automatically maybe?

For now, I will comment fr: and es: for nitrate

@stephanegigandet
Copy link
Contributor

@benbenben2 the expected test results can be updated automatically, by running make update_tests_results
I can do it if you want

Copy link
Contributor

@stephanegigandet stephanegigandet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, thanks a lot for updating the original PR! :)

@benbenben2
Copy link
Collaborator Author

I can do it if you want

I will try by myself and learn :)

@benbenben2 benbenben2 requested a review from a team as a code owner April 12, 2023 21:02
@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information

@benbenben2
Copy link
Collaborator Author

@stephanegigandet
Actually... if you have time, please do it in your side and let me know if you encounter any error.

So far, from my side. Running in Gitpod. Running:

make update_tests_results

After couple of minutes, it results to (not sure if it is because I am on gitpod, seems related to timeout of something...)

Waited too much for backend at /opt/product-opener/lib/ProductOpener/APITest.pm line 128.
        ProductOpener::APITest::wait_server() called at /opt/product-opener/lib/ProductOpener/APITest.pm line 141
        ProductOpener::APITest::wait_application_ready() called at integration/search_v1.t line 18
make: *** [Makefile:280: update_tests_results] Error 22

I aslo tried to follow guidelines from "how-to-write-and-run-tests.md", and tried:
make unit-test="producers.t --update-expected results"

Is it correct command? (Makefile file does not contain "make unit-test" but "make test-unit"). Hence, I also tried:
make test-unit test="producers.t --update-expected-results"

I eventually could see that nitrate appeared in both ES and FR files.

Then, commit and pushed. It result in new errors:

2023-04-12T21:21:26.9420434Z #          $got->{vitamin-b9-folic-acid-unprepared-per-serving-value} = HASH(0x5635a7ebba20)
2023-04-12T21:21:26.9420995Z #     $expected->{vitamin-b9-folic-acid-unprepared-per-serving-value} = Does not exist
2023-04-12T21:22:24.1718507Z #          $got->{unite-legumes-non-prepare-portion} = HASH(0x5635a89eecf0)
2023-04-12T21:22:24.1718974Z #     $expected->{unite-legumes-non-prepare-portion} = Does not exist
2023-04-12T21:23:22.6974950Z #          $got->{valor-gordo-monoinsaturadas-preparado-per-100g} = HASH(0x5635b1b15780)
2023-04-12T21:23:22.6975507Z #     $expected->{valor-gordo-monoinsaturadas-preparado-per-100g} = Does not exist

And here I'm stuck because changes are related to these nutrients in the error messages. But changes are for PT...

@stephanegigandet
Copy link
Contributor

@benbenben2 well the tests seem to be passing now, so we can merge

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🧬 Taxonomies https://wiki.openfoodfacts.org/Global_taxonomies
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants