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

Ability to write benchmark comparison results to file #18

Merged
merged 3 commits into from Feb 9, 2018

Conversation

davidchall
Copy link
Owner

@davidchall davidchall commented Feb 7, 2018

@michaeltryby I've added the ability to write the results of the benchmark comparison to a JSON file (closes #17). It can be activated at the command line via:

nrtest compare new/ old/ -o receipt.json

and the file looks like:

receipt.json
{
    "Application_Reference": {
        "description": "EPANET2012",
        "name": "EPANET",
        "version": "2.0.12"
    },
    "Application_UnderTest": {
        "description": "EPANET220",
        "name": "EPANET",
        "version": "2.2.0"
    },
    "CompareTolerance": {
        "absolute": 0.0,
        "relative": 0.01
    },
    "Tests": [
        {
            "description": "A simple example of modeling chlorine decay",
            "name": "Example 1",
            "passed": true
            "error_msg": null,
        },
        {
            "description": "Example of modeling a 55-hour fluoride tracer study.",
            "name": "Example 2",
            "passed": false,
            "error_msg": "Unexpected error occurred during diff"
        }
    ]
}

Additionally, the behavior when a comparison routine raises an exception has been adjusted. In the past, the exception message was printed to screen, accompanied by a second message Unexpected error occurred during diff. Now you will see a message Exception raised during diff: [exception message], which is also written in this new output file. So you can now raise exceptions in your custom comparison routine and see them appear in the receipt.json file, providing the details that you need for your own application. This essentially fixes #11.

The PR also fixes #16.

Please let me know if the file provides sufficient information for your purposes, and I can merge this.

@coveralls
Copy link

coveralls commented Feb 7, 2018

Coverage Status

Coverage decreased (-0.06%) to 28.297% when pulling 1c22669 on compare_logfile into 030719f on master.

@michaeltryby
Copy link

@davidchall this looks fantastic! The only thing I would add to the receipt.json is the comparison tolerances that were used. That way all the parameters of the test would be fully described. Thanks for the bug fixes too!

@davidchall
Copy link
Owner Author

@michaeltryby I can add the tolerances to this file for now.

But if #15 follows the route I’ve laid out in my latest comment, then these fields need to be removed in the future. I have no problem with this, since #15 breaks the UI in any case so needs the version to be bumped to 0.3.0.

@davidchall
Copy link
Owner Author

Ok, I added the tolerances as requested and have updated the example receipt.json above. Merging now.

@davidchall davidchall merged commit d62f7aa into master Feb 9, 2018
@davidchall davidchall deleted the compare_logfile branch February 9, 2018 21:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants