Fixed the -m option not working

[noahvsb]

Fixed an issue where there would be no output if there's no cache provided. (false)

Fixed part of an issue where the -m option doesn't work. I think the rest of the problem is not related to this repository, but to rdf-test-suite.js. (false, it was fully fixed)

[noahvsb]

I discovered that the first issue is only fixed when you pass the -m option.

[noahvsb]

a bit more context

my instance of the test suite

with -c

$ node bin\runner.js ..\comunica\engines\query-sparql\spec\sparql-engine.js https://comunica.github.io/manifest-ldf-tests/sparql/sparql-manifest.ttl -c cache/ -t simple
info: Caching enabled in C:\Users\noahv\Documents\rdf-test-suite-ldf.js\cache\
info: Loading objectloader for manifest creation
info: Importing manifest
info: Running manifest
info: Run test: https://comunica.github.io/manifest-ldf-tests/sparql/sparql-manifest.ttl#simple03
√ SELECT - DBPedia TPF & SPARQL (https://comunica.github.io/manifest-ldf-tests/sparql/sparql-manifest.ttl#simple03) 283.9149ms
√ 1 / 1 tests succeeded!

with -m, without -c

$ node bin\runner.js ..\comunica\engines\query-sparql\spec\sparql-engine.js https://comunica.github.io/manifest-ldf-tests/sparql/sparql-manifest.ttl -m https://comunica.github.io/manifest-ldf-tests/sparql/~C:\Users\noahv\Documents\manifest-ldf-tests\sparql\ -t simple
info: Loading objectloader for manifest creation
info: Importing manifest
info: Running manifest
info: Run test: https://comunica.github.io/manifest-ldf-tests/sparql/sparql-manifest.ttl#simple03
√ SELECT - DBPedia TPF & SPARQL (https://comunica.github.io/manifest-ldf-tests/sparql/sparql-manifest.ttl#simple03) 69.419ms
√ 1 / 1 tests succeeded!

with neither

$ node bin\runner.js ..\comunica\engines\query-sparql\spec\sparql-engine.js https://comunica.github.io/manifest-ldf-tests/sparql/sparql-manifest.ttl -t simple
info: Loading objectloader for manifest creation
info: Importing manifest

instance of the test suite acquired with yarn install rdf-test-suite-ldf

with -c

$ rdf-test-suite-ldf ..\comunica\engines\query-sparql\spec\sparql-engine.js https://comunica.github.io/manifest-ldf-tests/sparql/sparql-manifest.ttl -c cache/ -t simple
info: Caching enabled in C:\Users\noahv\Documents\rdf-test-suite-ldf.js\cache\
info: Loading objectloader for manifest creation
info: Importing manifest
info: Running manifest
info: Run test: https://comunica.github.io/manifest-ldf-tests/sparql/sparql-manifest.ttl#simple03
√ SELECT - DBPedia TPF & SPARQL (https://comunica.github.io/manifest-ldf-tests/sparql/sparql-manifest.ttl#simple03) 210.3904ms
√ 1 / 1 tests succeeded!

with -m, without -c

$ rdf-test-suite-ldf ..\comunica\engines\query-sparql\spec\sparql-engine.js https://comunica.github.io/manifest-ldf-tests/sparql/sparql-manifest.ttl -m https://comunica.github.io/manifest-ldf-tests/sparql/~C:\Users\noahv\Documents\manifest-ldf-tests\sparql\ -t simple
info: Loading objectloader for manifest creation
info: Importing manifest

with neither

$ rdf-test-suite-ldf ..\comunica\engines\query-sparql\spec\sparql-engine.js https://comunica.github.io/manifest-ldf-tests/sparql/sparql-manifest.ttl -t simple
info: Loading objectloader for manifest creation
info: Importing manifest

[noahvsb]

Possibly related to rubensworks/rdf-test-suite.js#55

[noahvsb]

Turns out that the -m option not working was succesfully fixed, but I made another stupid mistake that just made the results exactly the same.

So then this PR is just for the -m option fix, not the -c fix

[rubensworks]

Could you give some background on why these two changes are necessary?
It's great that they fix the problem, but I don't understand why they fix it.

For example, why can args.c be true?
And why is the shallow copy required.

[noahvsb]

For example, why can args.c be true?

Because of this line: cachePath = Path.join(process.cwd(), (args.c === true ? '.rdf-test-suite-cache/' : args.c));
I saw it supported argc.c being true, but it didn't allow this, since it doesn't end with a slash.

And why is the shallow copy required.

I'm not sure if you noticed, but before it was using urlToFileMapping, while it should've been using urlToFileMappings. That's why it didn't work, so I renamed it and did a shallow copy + the new option. But thinking back about it, just doing config.urlToFileMappings = this.fromUrlToMappingString(config.urlToFileMapping); would've also worked I think.

[rubensworks]

I saw it supported argc.c being true, but it didn't allow this, since it doesn't end with a slash.

What do you mean exactly?
Does it break because of the trailing slash there?
If so, maybe we just need to remove the slash there then?

[rubensworks]

I'm not sure if you noticed, but before it was using urlToFileMapping, while it should've been using urlToFileMappings

Aha, ok, good catch!

But thinking back about it, just doing config.urlToFileMappings = this.fromUrlToMappingString(config.urlToFileMapping); would've also worked I think.

It's fine either way :-)

[noahvsb]

What do you mean exactly?

If you just do -c without any string, then it defaults to .rdf-test-suite-cache/. But since there's no string, it produced TypeError: args.c.endsWith is not a function before. But now it works as intended, with the default path.

[rubensworks]

Ok, that's clear. Thanks!