jakartaee / jsonp-api

Jakarta JSON Processing
https://eclipse.org/ee4j/jsonp
Other
139 stars 59 forks source link

rename TCK package from package jakarta.jsonp.tck to jsonp.tck #351

Closed scottmarlow closed 2 years ago

scottmarlow commented 2 years ago

Signed-off-by: Scott Marlow smarlow@redhat.com

Copyright dates need to be updated and locally, I always see a test failure (with and without changes):

[INFO] Results:
[INFO] 
[ERROR] Errors: 
[ERROR]   ClientTests.jsonParserCurrentEvent:1862 NoSuchMethod 'jakarta.json.stream.Json...
[ERROR]   ClientTests.testDuplicateKeysFirst:3214 NoClassDefFound jakarta/json/JsonConfi...
[ERROR]   ClientTests.testDuplicateKeysLast:3233 NoClassDefFound jakarta/json/JsonConfig...
[ERROR]   ClientTests.testDuplicateKeysNone:3194 NoClassDefFound jakarta/json/JsonConfig...
[ERROR]   ClientTests.jsonNumber21Test:347 NoSuchMethod 'jakarta.json.JsonNumber jakarta...
[INFO] 
[ERROR] Tests run: 177, Failures: 0, Errors: 5, Skipped: 0
lukasj commented 2 years ago

rebasing on master should help

lukasj commented 2 years ago

wrt copyright updates, I run (on mac):

$ git add
$ for f in $(git diff --cached --name-only); do sed -E -i '' -e "s/([[:digit:]][[:digit:]][[:digit:]][[:digit:]]), ([[:digit:]][[:digit:]][[:digit:]][[:digit:]])/\1, 2022/" $f; done

eventually also an alternative for files created in 2021

scottmarlow commented 2 years ago

rebasing on master should help

Thanks, ^ addressed most of the failures, I'm seeing the below failure now with my changes:

[ERROR] jsonp.tck.pluggability.jsonprovidertests.ClientTests.jsonProviderTest1  Time elapsed: 0.045 s  <<< FAILURE!
org.opentest4j.AssertionFailedError: jsonProviderTest1 Failed ==> expected: <true> but was: <false>
        at org.junit.jupiter.api.AssertionUtils.fail(AssertionUtils.java:55)
        at org.junit.jupiter.api.AssertTrue.assertTrue(AssertTrue.java:40)
        at org.junit.jupiter.api.Assertions.assertTrue(Assertions.java:193)
        at jsonp.tck.pluggability.jsonprovidertests.ClientTests.jsonProviderTest1(ClientTests.java:105)
        at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
.
.
.
[INFO] 
[INFO] Results:
[INFO] 
[ERROR] Failures: 
[ERROR]   ClientTests.jsonProviderTest1:105 jsonProviderTest1 Failed ==> expected: <true> but was: <false>
[INFO] 
[ERROR] Tests run: 18, Failures: 1, Errors: 0, Skipped: 0

I don't get the ^ failure if I switch to the master branch, build the TCK and test again.

From impl-tck/tck-tests-pluggability/target/surefire-reports/TEST-jsonp.tck.pluggability.jsonprovidertests.ClientTests.xmloutput logged after the test:

Jan 12, 2022 10:27:44 AM jsonp.tck.pluggability.jsonprovidertests.ClientTests jsonProviderTest1
INFO: provider class=jsonp.tck.provider.MyJsonProvider
Jan 12, 2022 10:27:44 AM jsonp.tck.pluggability.jsonprovidertests.ClientTests jsonProviderTest1
WARNING: Current provider is not my provider - unexpected.
Jan 12, 2022 10:27:44 AM jsonp.tck.pluggability.jsonprovidertests.ClientTests jsonProviderTest1
INFO: Providers: [jsonp.tck.provider.MyJsonProvider@7180e701, org.eclipse.parsson.JsonProviderImpl@4e2c95ee]
scottmarlow commented 2 years ago

I updated the information logged to show:

LOGGER.warning("Current provider is not my provider - unexpected.  providerClass=" + providerClass + " , MY_JSONPROVIDER_CLASS=" +MY_JSONPROVIDER_CLASS);

^ shows:


WARNING: Current provider is not my provider - unexpected.  providerClass=jsonp.tck.provider.MyJsonProvider , MY_JSONPROVIDER_CLASS=MyJsonProvider```
scottmarlow commented 2 years ago

I now see the problem and will fix it, I had changed:

private static final String MY_JSONPROVIDER_CLASS = "jakarta.jsonp.tck.provider.MyJsonProvider";

To:

private static final String MY_JSONPROVIDER_CLASS = "MyJsonProvider";

Which caused the failure.

scottmarlow commented 2 years ago

wrt copyright updates, I run (on mac):

$ git add
$ for f in $(git diff --cached --name-only); do sed -E -i '' -e "s/([[:digit:]][[:digit:]][[:digit:]][[:digit:]]), ([[:digit:]][[:digit:]][[:digit:]][[:digit:]])/\1, 2022/" $f; done

eventually also an alternative for files created in 2021

Thanks, I made a note of ^

I used https://github.com/eclipse-ee4j/glassfish-copyright-plugin (clone it and build it locally):

export COMMITID=83c67278c583e5645cfe77f936396277b00ccc6c
repo=~/.m2/repository/org/glassfish/copyright/glassfish-copyright-maven-plugin
v=`ls $repo | grep '^[1-9]' | tail -1`
for i in `git diff-tree --no-commit-id --name-only -r $COMMITID`; do echo java -cp $repo/$v/glassfish-copyright-maven-plugin-$v.jar org.glassfish.copyright.Copyright -r -c $i >> /tmp/script.sh; done
chmod +x /tmp/script.sh
. /tmp/script.sh

I also did:

export COMMITID=83c67278c583e5645cfe77f936396277b00ccc6c
repo=~/.m2/repository/org/glassfish/copyright/glassfish-copyright-maven-plugin
v=`ls $repo | grep '^[1-9]' | tail -1`
for i in `git diff-tree --diff-filter=RA --no-commit-id --name-only -r $COMMITID`; do echo java -cp $repo/$v/glassfish-copyright-maven-plugin-$v.jar org.glassfish.copyright.Copyright -r -c $i >> /tmp/script.sh; done
chmod +x /tmp/script.sh
. /tmp/script.sh

After doing the above two steps, I reviewed what was updated and reverted a few unneeded changes where a copyright was added to a file that didn't have one before (e.g. signature map file).

scottmarlow commented 2 years ago

I'm trying to run on master branch with latest changes locally and seeing failure:

Failures: 
[ERROR]   JsonProviderTest.systemProperty:57 expected: <jakarta.jsonp.tck.api.provider.JsonProviderTest.DummyJsonProvider> but was: <org.eclipse.parsson.JsonProviderImpl>
scottmarlow commented 2 years ago

I'm going to create a new pr that uses the ee.jakarta.tck package since that was used for https://github.com/eclipse-ee4j/jsonb-api

lukasj commented 2 years ago

what exactly are you trying to run? just rebase, build the tck, unzip the binary and run bin/pom.xml - the force will guide you (or user guide? who knows ;-) ); ignore the impl-tck folder

scottmarlow commented 2 years ago

Replaced by https://github.com/eclipse-ee4j/jsonp/pull/355

scottmarlow commented 2 years ago

what exactly are you trying to run? just rebase, build the tck, unzip the binary and run bin/pom.xml - the force will guide you (or user guide? who knows ;-) ); ignore the impl-tck folder

I had done something like:

cd api;mvn clean install;cd ../tck;mvn clean install;cd ../impl-tck;mvn clean install

Thanks, I will ignore the impl-tck folder