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

Tweak PDFContentImporter #945

Merged
merged 1 commit into from
Mar 13, 2016
Merged

Tweak PDFContentImporter #945

merged 1 commit into from
Mar 13, 2016

Conversation

koppor
Copy link
Member

@koppor koppor commented Mar 12, 2016

This is a minor update on PdfContentImporter, which fixes minor issues and enables a broader test.

// The test folder contains pairs of PDFs and BibTeX files. We check each pair.
// Currently only one pair available
List<String> prefixes = Arrays.asList("LNCS-minimal");
Copy link
Member

Choose a reason for hiding this comment

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

I wouldn't write this as a for loop and use parametrized tests instead.

Copy link
Member Author

Choose a reason for hiding this comment

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

Like

public void testImport(sourcePdfName, shouldBeBibFileName) {
...
}

public void testLNCS() {
    testImport("LNCS-minimal.pdf", "LNCS-minimal.bib")
}

Copy link
Member

Choose a reason for hiding this comment

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

Like https://github.com/junit-team/junit/wiki/Parameterized-tests and see the FormatterTest for an example.

Copy link
Member Author

Choose a reason for hiding this comment

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

Done.

@oscargus
Copy link
Contributor

👍

@simonharrer
Copy link
Contributor

👍 rebase and merge

 * Do not fill review field any more - this was used for debugging
 * Do not write empty keyword field
 * Remove newline at the end of abstract
koppor added a commit that referenced this pull request Mar 13, 2016
@koppor koppor merged commit 854c090 into master Mar 13, 2016
@koppor koppor deleted the tweakPdfContentImporter branch March 13, 2016 13:26
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.

None yet

4 participants