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

RISImporterTest [v2] #817

Merged
merged 1 commit into from
Mar 18, 2016
Merged

Conversation

obraliar
Copy link
Contributor

No description provided.

@obraliar
Copy link
Contributor Author

obraliar commented Mar 4, 2016

All requested test cases from #505 have been successfully implemented.

@obraliar obraliar changed the title [WIP] RISImporterTest [v2] RISImporterTest [v2] Mar 10, 2016
@boceckts boceckts added the status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers label Mar 11, 2016
@obraliar
Copy link
Contributor Author

Ready to merge?

Assert.assertTrue(importer.isRecognizedFormat(stream));
}
try (InputStream stream = RISImporterTest.class.getResourceAsStream("RisImporterTest2.ris")) {
Copy link
Member

Choose a reason for hiding this comment

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

Move to new test method

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.


int referenceCount = 0;
for (int i = 0; i < risEntries.size(); i++) {
for (int j = 0; j < bibEntries.size(); j++) {
Copy link
Member

Choose a reason for hiding this comment

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

Wait until #982 is merged in, then a simpler BibtexEntryAssert.assertEquals() should work.

public class RISImporterTest {

RisImporter risImporter;
Copy link
Contributor

Choose a reason for hiding this comment

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

make this private

@simonharrer
Copy link
Contributor

👍 LGTM

@obraliar
Copy link
Contributor Author

Prepare for merge (merge commits)?

@tobiasdiez
Copy link
Member

LGTM, too.
Two last remarks:

  • RisImporterTest2.bib is not used, right? Delete it.
  • Rename RisImporterTest2.ris to RisImporterCorrupted.ris (since this what the file represents, isn't it?)

Finally squash all commit together then this can be merged in.

@obraliar
Copy link
Contributor Author

Done.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants