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

Documenter testcases #281

Merged
merged 5 commits into from
May 26, 2022
Merged

Conversation

navinrathore
Copy link
Contributor

Please provide inputs.

@BeforeEach
public void setUp(){
try {
args = Arguments.createArgumentsFromJSON(getClass().getResource("/testConfig.json").getFile());
Copy link
Member

Choose a reason for hiding this comment

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

please put configs in respective folders - tests/resources/documenter.config..json

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

args = Arguments.createArgumentsFromJSON(getClass().getResource("/testConfig.json").getFile());
} catch (Throwable e) {
e.printStackTrace();
System.out.println("Unexpected exception received " + e.getMessage());
Copy link
Member

Choose a reason for hiding this comment

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

use logger

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

Dataset<Row> data = PipeUtil.read(spark, false, false, args.getData());
f.invoke(dataColDoc, data);

assertTrue(Files.exists(Paths.get(file1)), "Data col documentation add1.html is not generated");
Copy link
Member

Choose a reason for hiding this comment

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

other than file path, is there a way to test other methods ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The template is verified at compile time itself. Generated html/csv file unittest may not help. We can add some way of verification, if we get issues in future.

https://stackoverflow.com/questions/3429218/unit-tests-for-html-output

public void testProcessTemplateToMakeDocument() throws Throwable {

DocumenterBase base = new DocumenterBase(spark, args);
Method f = DocumenterBase.class.getDeclaredMethod("writeDocument", String.class, Map.class, String.class);
Copy link
Member

Choose a reason for hiding this comment

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

same comment as above

Copy link
Contributor Author

Choose a reason for hiding this comment

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

updated above.

@@ -38,7 +37,7 @@ public void process() throws ZinggClientException {
modelColDoc.process(markedRecords);
}

private void createModelDocument() throws ZinggClientException {
protected void createModelDocument() throws ZinggClientException {
Copy link
Member

Choose a reason for hiding this comment

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

we should separate the model from the view as in mvc architecture - one method to populate the data, and one to write the view. that will be far easier to test - you can send dataframes and verify that all that you want to display on the page is getting populated correctly in the map. the file writing is not so much of a concern as freemarker is taking care of it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Broken into two functions - populateTemplateData() and writeDataDocument()
Added 1 new test case.

@@ -54,7 +54,7 @@ public void process() throws ZinggClientException {
}
}

private void createDataDocument() throws ZinggClientException {
protected void createDataDocument() throws ZinggClientException {
Copy link
Member

Choose a reason for hiding this comment

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

if you break this method into one method that builds the map and second that uses the map and template to write it, you can test far more cases.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Broken into two functions - populateTemplateData() and writeModelDocument();
Added 2 testcases.

@navinrathore
Copy link
Contributor Author

moved template file into documenter resource folder

@sonalgoyal sonalgoyal merged commit 16f16c4 into zinggAI:main May 26, 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.

2 participants