Repository navigation
Java uses the default JSON endpoint and still accepts the old one - #23
Draft
jwrosewell wants to merge 1 commit into
Draft
jwrosewell wants to merge 1 commit into
jwrosewell wants to merge 1 commit into
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
The Java descriptor in
Examples/ExampleApps.cssetsJsonEndpointPathto/51Degrees.core.json. The specification's path is/51dpipeline/json, which .NET and Go use and which is this suite's default. 51degrees-java draft #80 (head 2d741ef6) makes Java default to/51dpipeline/jsonand still answer the old path, and the script it serves calls/51dpipeline/json. With the old override the session storage cache test proxies and counts only/51Degrees.core.json, so Java #80's full build fails with "the first page must call the json endpoint at least once".Change
JsonEndpointPath, so Java uses the default like the other languages.ExampleDescriptorgainsAlternativeJsonEndpointPaths, older paths an integration still answers. Java lists/51Degrees.core.jsonthere.ExampleApps.JsonEndpointPathsreturns the default plus those alternatives.TestHelpers.ExampleProxyListenerproxies every path in the list, and the session storage cache test counts POSTs to any of them.The suite therefore passes against Java
rc(script calls/51Degrees.core.json) and against Java #80 (script calls/51dpipeline/json). The merge order does not matter, and the alternative path can be removed once Java #80 has reachedrcandmain.How this was checked
Java's CI clones this repository's
maininci/run-integration-tests.ps1(line 97), honouring an environment variableSELENIUM_TESTS_REF. Nothing in common-ci'snightly-pull-request.ymlor Java's workflows sets that variable or accepts one, so Java #80's full build cannot be pointed at this branch without changing 51degrees-java or merging here. It has not been run against this branch. This repository has no secrets, so its own workflow cannot run the Contract tests either.This repository's own Build and test workflow runs on this pull request and proves the change compiles and the non-browser tests pass.
Remaining
After merge, rerun Java #80's full build (and Java
rc's) to confirm the contract tests pass with both script paths.