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

fix: Add tests for Carrefour France import, + solve serving_size bug #6476

Merged
merged 9 commits into from
Mar 11, 2022

Conversation

stephanegigandet
Copy link
Contributor

@stephanegigandet stephanegigandet commented Mar 9, 2022

There's a long standing issue #2973 that results in almost daily changes being made to some products imported by Carrefour France. The corresponding input files in fact did not change, but we were mapping multiple values (for different languages) to the same field: the values were concatenated, but in a not specified order. So the values changed.

This PR:

  • adds some tests so that we can catch those issues, and also so that we can more easily improve the Carrefour data conversion (if needed)
  • uses only the French value for fields like the quantity (we could try to support more cases, but the extra logic may not be worth the very few cases where that might be useful)
  • solves some small issues, undefined values etc.

Note: in order to be able to create a test, most of the code from convert_carrefour_data.pl was moved to a new ImportConvertCarrefourFrance.pm module, but no attempt was made to improve that code.

@stephanegigandet stephanegigandet requested a review from a team as a code owner March 9, 2022 17:37
@github-actions github-actions bot added Data import export 🏭 Producers Platform https://wiki.openfoodfacts.org/Platform_for_producers Products 🧪 tests labels Mar 9, 2022
@stephanegigandet stephanegigandet changed the title Add tests for Carrefour France import, + solve serving_size bug fix: Add tests for Carrefour France import, + solve serving_size bug Mar 9, 2022
ProductOpener::Test::compare_csv_file_to_expected_results($exported_csv_file, $test_dir . "/expected_test_results/import_convert_carrefour_france_export", $update_expected_results);


done_testing();
Copy link
Member

Choose a reason for hiding this comment

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

Really cool to have simple integrations tests like this.

Copy link
Member

@alexgarel alexgarel left a comment

Choose a reason for hiding this comment

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

I think you forgot to add ImportConvertCarrefourFrance.pm @stephanegigandet !

Copy link
Member

@alexgarel alexgarel left a comment

Choose a reason for hiding this comment

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

LGTM

@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

@stephanegigandet stephanegigandet merged commit f255f30 into main Mar 11, 2022
@stephanegigandet stephanegigandet deleted the import-serving-size-bug branch March 11, 2022 10:21
@teolemon teolemon added this to the V1.1.0 milestone Apr 1, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Data import export 🏭 Producers Platform https://wiki.openfoodfacts.org/Platform_for_producers Products 🧪 tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants