Empty prefix support

[coreation]

This fixes issue #181, tested with a JSON output and a Turtle output (the latter really shows the difference and correctness). If any more testing needs to be done before this pull request can happen I'd be glad to look if I can be of assistance.

[indeyets]

@coreation can you add corresponding unit-tests, please?

[coreation]

Ofcourse, this test should basically add an empty prefix, and check (when a triple is added with an empty prefix) if it is lited in the @Prefix turtle output? Anything else I should add to it?

[indeyets]

Yup.

1. The test should fail if ran without your patch
2. The test should pass if ran with your patch
3. Turtle document produced by patched code (the one used in test) should pass validation (no need to automate it — manual test against legitimate external validator is enough)
4. Amount of failed tests (we have some) should not increase with your patch

[coreation]

Ok, I'll run the tests before I add mine, to assess how many test normally fail ;).

[coreation]

@indeyets I don't seem to get a unittest working, I installed it through composer (and updated), yet if I run any test I get:

PHP Warning: require_once(PHPUnit/Framework/IncompleteTestError.php): failed to open stream: No such file or directory in /path/to/easyrdf/test/TestHelper.php on line 41

If I look at my autoload classmap file, I see the class reference (be it with _ instead of folder separation but that shouldn't form a problem?) I run php 5.4.

[indeyets]

@coreation are you sure you're on "devel" branch? it looks like you have phpunit4 installed. That can happen on "master", but devel is fixed to require phpunit3

[coreation]

Well I'm on the "patch-1 branch" since I didn't for the repository myself. Looking at the network, I see that the patch-1 branch has been forked from the devel branch. Just to be sure I checked out the devel branch, composer update didn't update any dependencies and this:

php -v test/examples/BasicTest.php

still gives this error:

PHP Warning: require_once(PHPUnit/Framework/IncompleteTestError.php): failed to open stream: No such file or directory in /path/to/easyrdf/test/TestHelper.php on line 41

[indeyets]

@coreation ok. that's not how you run tests.

full test suite: make test

just the tests you should care about: ./vendor/bin/phpunit test/EasyRdf

[coreation]

Ah, sorry I'm used to having a phpunit.xml file which does that for me ;).

[coreation]

@indeyets Ok, so I did the following:

1. made sure the tests failed with my patch (two tests failed in the NamespaceTest -> which is to be expected as they were both dealing with empty namespaces.
2. Rewrote the test functions, and ran them with my patch -> tests came out positive
3. turtle document is valid (ran against external validator)
4. As the only file it affected was the file that previously gave 2 errors in step 1) and now no errors in step 2) + the global test test/EasyRdf gives no more failures than the original devel branch did I think I've provided enough unittesting.

If there's anything more I should contribute on this issue let me know.

[indeyets]

@coreation Can you make related turtle-serializer test? that's an interesting part. current test doesn't create any document, so I'm not sure what you tested against external validator.

[indeyets]

2 lines above are not needed

[coreation]

You mean this function can be removed from the test?

[indeyets]

nope. I mean that 2 empty lines are not needed :)

[coreation]

So in the Turtle Serializer test I should add something in the same line as this function:

 public function testSerialise()
    {
        $joe = $this->graph->resource(
            'http://example.com/joe#me',
            'foaf:Person'
        );
        $joe->set('foaf:name', 'Joe Bloggs');
        $joe->set(
            'foaf:homepage',
            $this->graph->resource('http://example.com/joe/')
        );

        $turtle = $this->serialiser->serialise($this->graph, 'turtle');
        $this->assertSame(
            "@prefix foaf: <http://xmlns.com/foaf/0.1/> .\n".
            "\n".
            "<http://example.com/joe#me>\n".
            "  a foaf:Person ;\n".
            "  foaf:name \"Joe Bloggs\" ;\n".
            "  foaf:homepage <http://example.com/joe/> .\n\n",
            $turtle
        );
    }

But then for an empty namespace prefix?

[coreation]

@indeyets provided the extra unittest in the Turtle seriliazer, and removed the 2 white lines.

[indeyets]

@coreation thank you. l hope to check/merge it later today. I just understood, that we also need to validate that it doesn't break other serialisers

[coreation]

@indeyets No problem, so other serialisers might break because you can add an empty namespace? Makes sense, since that premisse was never present so far. Keep me posted.

[coreation]

@indeyets Do you need me to write a serialise test for the other serialisers with an empty prefix as well? Or is that taken care of?

[indeyets]

@coreation if you have some time — that would be cool. I just can't fine a minute myself :-(

[coreation]

I'll look into it the coming days.

On Wed, Apr 23, 2014 at 10:10 AM, Alexey Zakhlestin <
notifications@github.com> wrote:

@coreation https://github.com/coreation if you have some time — that
would be cool. I just can't fine a minute myself :-(

—
Reply to this email directly or view it on GitHubhttps://github.com//pull/183#issuecomment-41135100
.

Met vriendelijke groeten,

Jan Vansteenlandt

[coreation]

Added the empty namespace test in most of the serializer tests. The ones I didn't edit, were the ones I think that don't need extra testing. Could be wrong ofcourse.

[indeyets]

@coreation I modified your RDF/XML test a bit and we clearly have a breakage.

diff --git a/test/EasyRdf/Serialiser/RdfXmlTest.php b/test/EasyRdf/Serialiser/RdfXmlTest.php
index 2f7f228..7464795 100644
--- a/test/EasyRdf/Serialiser/RdfXmlTest.php
+++ b/test/EasyRdf/Serialiser/RdfXmlTest.php
@@ -426,6 +426,7 @@ class EasyRdf_Serialiser_RdfXmlTest extends EasyRdf_TestCase
             'foaf:homepage',
             $this->graph->resource('http://example.com/joe/')
         );
+        $joe->set('http://foo/bar/test', 'test');

         $rdf = $this->serialiser->serialise($this->graph, 'rdfxml');

produces this invalid document:

<?xml version="1.0" encoding="utf-8" ?>
<rdf:RDF xmlns:rdf="http://www.w3.org/1999/02/22-rdf-syntax-ns#"
         xmlns:foaf="http://xmlns.com/foaf/0.1/"
         xmlns:="http://foo/bar/">

  <foaf:Person rdf:about="http://foo/bar/me">
    <foaf:name>Joe Bloggs</foaf:name>
    <foaf:homepage rdf:resource="http://example.com/joe/"/>
    <:test>test</:test>
  </foaf:Person>

</rdf:RDF>

[indeyets]

A part of the issue is, that currently EasyRdf_Serialiser_RdfXml impements ad-hoc XML generator, instead of using a proven base.

I'll try to recreate it using XMLWriter. @njh what do you think about it?

[coreation]

Okay, that clearly breaks it indeed. If you need anything from me, let me know.

[coreation]

Any update to be fetched on this? :)

[pietercolpaert]

nudge?

[indeyets]

merged