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

49 junit integration #51

Merged
merged 27 commits into from
Sep 24, 2015
Merged

49 junit integration #51

merged 27 commits into from
Sep 24, 2015

Conversation

pepperbob
Copy link
Contributor

Hi Dimitris,

we started implementing the Runner and so far it looks quite promising. Though we need to add some features (CVs) and improve overall performance.

Maybe you can already have a glance. Feel free to comment.

Regards,
Daniel & Michael

Michael Leuthold and others added 13 commits September 21, 2015 16:12
Initial Test Runner that reads out an ontology and converts it into
test cases, needs further description of test cases and their execution.
We should think about using a different Exception?
Created first implementation of test case invocation, not called though
First version of tests run, needs optimization of ontology retrieval
Optimization - Single model creation for ontology
Optimization - Create input model only once
Minor refactoring: cleaned up call mess
Changed example test to being executed via Request, and then checking
whether there are failed and succeeded tests.
Updated test description to include input model method name and
test uri.
Added resources to test failure description.
Extracted RLOGStatement to own class.
Controlled vocabulary integration.
@@ -108,7 +114,8 @@ public Builder addSchemaURI(String prefix, String schemaUri) {
public Builder addLocalResource(String prefix, String localResource) {
checkNotNull(localResource);

SchemaSource schemaSource = createSource(prefix, localResource, RDFReaderFactory.createResourceReader(localResource));
SchemaSource schemaSource = createSource(prefix, localResource, RDFReaderFactory.createResourceReader
(localResource));
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We should undo changes in this file up to here as it's formatter related.

@jimkont
Copy link
Member

jimkont commented Sep 23, 2015

Great work Daniel & Michael!
Some high level comments (for now).

  1. I would rather use the term schema instead of ontology because it can cover other schema types that RDFUnit currently supports (or will support in the future, like shacl).

  2. the file paths that are used sometimes cause initialization errors when testing from the IDE (Intellj). Maybe it is better to use the java resource naming directly

instead of Model as input we could use RDFReaders or SchemaSource/TestSource that can more transparently handle java resources.

  1. I created an example test based on your code but is not picked up my maven. (although I can run it from Intellj when I set the relative paths correctly).
@RunWith(RdfUnitJunitRunner.class)
@Ontology(uri = Constants.FOAF_ONTOLOGY_URI)
public class ExampleRDFUnitRunner {

        @InputModel
        public Model getInputData() {
            return ModelFactory
                    .createDefaultModel()
                    .read("file:src/test/resources/inputmodels/foaf.rdf");
        }
}
  1. re-running only failed tests (supported by the IDE) does not work but this is minor

  2. we could also allow a list of @InputModel and the runner could create test combinations of schema x InputModel

  3. Test descriptions could use some tweaking but I need to deal with some prefix generation related tasks first (e.g. Remove the dependency to LOV for every validation #34 / More transparent facilites to specify local alternatives for LOV schema locations for CLI #39)

overall, I am very happy to accept it as is and iterate over time but I see you made some comments already. Let me know when you are ready to merge

clean code / PR issues
Collection<TestCaseResult> runTest(RdfUnitJunitTestCase rdfUnitJunitTestCase)
throws IllegalAccessException, InvocationTargetException {
final TestSource modelSource = new TestSourceBuilder()
.setPrefixUri("custom", "rdfunit")
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@jimkont Could you explain what's the thing with the Prefix URI? Just noticed we just define constant / hard coded values to satifsfy the Builder.. However not sure if this is correct.

Copy link
Member

Choose a reason for hiding this comment

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

prefix are used as in turtle syntax (e.g. http://prefix.cc/popular.file.txt) and are meant to shorten the IRIs or as secondary (short) identifiers.
I also use them to override the actual schema IRI when I want to use a different ontology IRI than the default one
e.g. nif https://github.com/AKSW/RDFUnit/blob/master/data/schemaDecl.csv#L12 skips the default dereferencing and uses the latest version from github.
this way when the user checks against e.g. nif the the overridden IRI is used transparently

If we use a SchemaSource and a TestSource directly in the tests definitions, instead of Jena Models, then we can reuse the prefixes defined in the constructors and avoid the hardcoded prefixes

Copy link
Member

Choose a reason for hiding this comment

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

In this case we cannot override anything by defining a prefix but the prefix can be used to improve the readability of the test description

Copy link
Contributor

Choose a reason for hiding this comment

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

OK, so what would you propose regarding the setPrefixUri() call?

Copy link
Member

Choose a reason for hiding this comment

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

To get rid off hardcoding stuff we should go with an approach similar to


@RunWith(RdfUnitJunitRunner.class)
public class ExampleTest {

    @Schema
    public SchemaSource schemaSource =
SchemaSourceFactory.createSchemaSourceSimple("foaf",
Constants.FOAF_ONTOLOGY_URI,
RDFReaderFactory.createResourceReader(Constants.FOAF_ONTOLOGY_URI));


    @TestInput
    public TestSource testSource = new TestSourceBuilder()
            .setImMemSingle()
            .setInMemReader(RDFReaderFactory.createResourceReader("inputmodels/foaf.rdf"))
            .setReferenceSchemata(schemaSource)
            .build();
}

and we can define some factory methods to simplify object construction

On Wed, Sep 23, 2015 at 12:24 PM, Daniel Hiller notifications@github.com
wrote:

In
rdfunit-junit/src/main/java/org/aksw/rdfunit/junit/RdfUnitJunitStatusTestExecutor.java
#51 (comment):

+import org.aksw.rdfunit.sources.TestSource;
+import org.aksw.rdfunit.sources.TestSourceBuilder;
+import org.aksw.rdfunit.tests.executors.RLOGTestExecutor;
+import org.aksw.rdfunit.tests.query_generation.QueryGenerationSelectFactory;
+import org.aksw.rdfunit.tests.results.TestCaseResult;
+
+final class RdfUnitJunitStatusTestExecutor extends RLOGTestExecutor {
+

  • public RdfUnitJunitStatusTestExecutor() {
  •    super(new QueryGenerationSelectFactory());
    
  • }
  • Collection runTest(RdfUnitJunitTestCase rdfUnitJunitTestCase)
  •        throws IllegalAccessException, InvocationTargetException {
    
  •    final TestSource modelSource = new TestSourceBuilder()
    
  •            .setPrefixUri("custom", "rdfunit")
    

OK, so what would you propose regarding the setPrefixUri() call?


Reply to this email directly or view it on GitHub
https://github.com/AKSW/RDFUnit/pull/51/files#r40192614.

Kontokostas Dimitris

Daniel Hiller added 7 commits September 23, 2015 10:35
- Removed unnecessary setPrefix()
- Changed from "file:" uri to no extension to use resource mechanism
- Moved TestSource building to test case generation
- Need to remove the unnecessary parameters from test case wrapper
Removed not needed SchemaSource from test case wrapper
- Renamed @InputModel to @TestInput as return type will be changed from Model
  to RdfReader
- Renamed @Ontology to @Schema to express more broad validation sources
Extracted InitializationSupport to reduce duplicate code on
initialization checks.
Fixed method and variable naming after renaming to "TestInput"
- Changed @TestInput return type from Model to RDFReader
- Changed @COntrolledVocabulary return type from Model to RDFReader
Daniel Hiller added 3 commits September 23, 2015 12:18
- Reverted accidental format change
Changed boolean value expression by extracting method
@dhiller
Copy link
Contributor

dhiller commented Sep 23, 2015

Think we've addressed most of the issues now.

Regarding the "re-run failed tests" issue, I believe this won't be achievable, though :(

@dhiller
Copy link
Contributor

dhiller commented Sep 23, 2015

@jimkont please let us know what you think :)

@dhiller
Copy link
Contributor

dhiller commented Sep 23, 2015

BTW regarding the "run-test" issue you mentioned, I use intellij to run the internal test contained in RunnerTest$TestRunner, works fine for me.

add Readme
add test for @BeforeClass and @afterclass
@jimkont
Copy link
Member

jimkont commented Sep 23, 2015

To get rid off hardcoding stuff we should go with an approach similar to

@RunWith(RdfUnitJunitRunner.class)
public class ExampleTest {

    @Schema
    public SchemaSource schemaSource = SchemaSourceFactory.createSchemaSourceSimple("foaf", Constants.FOAF_ONTOLOGY_URI, RDFReaderFactory.createResourceReader(Constants.FOAF_ONTOLOGY_URI));

    @TestInput
    public TestSource testSource = new TestSourceBuilder()
            .setImMemSingle()
            .setInMemReader(RDFReaderFactory.createResourceReader("inputmodels/foaf.rdf"))
            .setReferenceSchemata(schemaSource)
            .build();
}

and we can define some new factory methods to simplify object construction

@Schema(uri = "ontologies/foaf.rdf")
public static class TestRunner {

@ControlledVocabulary
Copy link
Member

Choose a reason for hiding this comment

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

It is not clear what @ControlledVocabulary does

Copy link
Member

Choose a reason for hiding this comment

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

If it is meant to be loaded along with the data (which is what I guess from the code) should be renamed to something like @AdditionalData but this can also be constructed directly on the @InputModel

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good Point - 'ControlledVocabulary' is just our use case (as we're loading CVs as additional data) but in general terms @AdditionalData is more suited.

In fact it gets merged with each InputModel later, but we need to keep both separated to figure out if the offending resources found during test execution is actually part of the InputModel or not.

Michael Leuthold added 2 commits September 23, 2015 20:01
rename @ControlledVocabulary to @AdditionalData
Run initial test validation through collectInitializationErrors
as more validation is up to come and needs separation from
setup code.

Maybe we should use dedicated Validators.
@dhiller
Copy link
Contributor

dhiller commented Sep 24, 2015

@jimkont Regarding your SchemaSource change proposal, we can't see how this would help as we need the RDFReader for test generation which is not exposed by the SchemaSource.

Aside from the initialization error handling (which could be improved) we would propose to rather merge this pull request to prepare for a release as it covers our current use cases. We would then create an issue pulling together the remaining improvements and get back when we can spend the time again.

Would that be ok for you?

jimkont added a commit that referenced this pull request Sep 24, 2015
@jimkont jimkont merged commit 4effd4d into AKSW:master Sep 24, 2015
@jimkont
Copy link
Member

jimkont commented Sep 24, 2015

more than happy. do you want me to make a new release?

@dhiller
Copy link
Contributor

dhiller commented Sep 24, 2015

We'd be glad if you could release this :)

@dhiller dhiller deleted the 49-junit-integration branch September 24, 2015 07:58
@jimkont
Copy link
Member

jimkont commented Sep 24, 2015

Thanks again for the amazing work! the release is ready
Also note that rdfunit is now in maven central http://search.maven.org/#artifactdetails|org.aksw.rdfunit|rdfunit-junit|0.7.19|

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

3 participants