Skip to content

Conversation

@j-sal
Copy link
Collaborator

@j-sal j-sal commented Oct 20, 2022

Fixes #75

Copy link
Member

@wetneb wetneb 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 to me! You could add a few assertions in some existing test, too.

Assert.assertEquals(project.columnModel.columns.get(1).getName(), "M-ids");
Assert.assertEquals(project.columnModel.columns.get(2).getName(), "Categories");
Assert.assertEquals(cell.recon.match.id, "M114916202");
Assert.assertEquals(cell.recon.match.name, "File:2006-08-08 Vista Parque Nacional Volcan Arenal, Costa Rica.jpg");
Copy link
Member

Choose a reason for hiding this comment

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

Very nice! One problem though: this test case seems to be fetching data from Wikimedia Commons without any mocking, so it is likely that the test will start failing whenever the contents of this category change. I would rather mock the API call with MockWebServer as in other tests. This might require a method to set the MediaWiki API endpoint to use in CommonsImporter.

Copy link
Member

@wetneb wetneb left a comment

Choose a reason for hiding this comment

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

Brilliant!

@j-sal j-sal merged commit fe483d3 into OpenRefine:master Oct 31, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Make column names meaningful upon project start

2 participants