Skip to content

TEST: Run the cloud tests again, and let a property have no value - #178

Draft
jwrosewell wants to merge 1 commit into
mainfrom
test/turn-the-cloud-tests-back-on
Draft

jwrosewell wants to merge 1 commit into
mainfrom
test/turn-the-cloud-tests-back-on

Conversation

@jwrosewell

@jwrosewell jwrosewell commented Sep 17, 2026

Copy link
Copy Markdown
Contributor

What is wrong

ValueCloudTests and MissingPropertyHandlingTests both carry a class level
@Ignore, added on 28 May 2025 in commit bc5559c, "TEST: Ignore cloud tests",
with no reason recorded. Six tests have not run since, anywhere, and the jobs
have reported success the whole time. They are the only tests this repository
has for reading values out of a cloud answer.

Turning them back on showed two real problems.

  1. The fixture in MissingPropertyHandlingTests names the element
    ip-intelligence. The engine reads ip, which is what
    IPIntelligenceCloudEngine.getElementDataKey() returns and what the service
    sends, so both tests threw JSONObject["ip"] not found.
  2. ValueTests.valueTypes asks every listed property for its value and its
    type. A property the resource key is not entitled to, or one the client
    script populates, is listed but carries no value and gives a reason instead,
    and asking such a value for its type throws. With a paid key the failure is
    ip is a paid feature.

The change

  • Both classes run again.
  • The fixture names the element ip.
  • ValueTests.valueTypes reads hasValue() first. Where there is no value it
    checks that the reason was given, which is what a caller reads, and moves on.
    Where there is a value the type check is unchanged. Nothing is skipped
    silently.

How it was checked

mvn -pl ip-intelligence.cloud -am test against https://cloud.51degrees.com
with TestResourceKey set to a paid key, on 17 September 2026:

  • with the @Ignore removed and nothing else changed,
    Tests run: 12, Failures: 0, Errors: 6, Skipped: 0, the six errors being the
    two problems above;
  • with this change, Tests run: 12, Failures: 0, Errors: 0, Skipped: 0.

ValueCloudTests needs evidence.csv from the data repository, which the
integration job already fetches.

Still outstanding

CloudRequestOriginTests.ResourceKeyWithOrigin carries its own @Ignore and a
resource key written into the source. That one needs a decision about which key
it should use before it can run, so it is left alone here.

The pull request and nightly jobs of this repository pass no test resource key
to the shared workflow, so the cloud tests that read one still ask to be skipped
in CI even with this change. That is covered in
#177, and adding
the secret is a change for a person to make.

The same change in the other repositories

device-detection-java#590
makes the same change to both copies of ValueTests.valueTypes there, where
the same walk hits a property the 51Degrees client script populates.

Why this is still a draft, 17 September 2026

Three things stand between this branch and a green run, and none of them is in
this change.

  1. The examples clone. ci/run-integration-tests.ps1 still clones
    ip-intelligence-java-examples at this branch's own name, which does not
    exist there, so the step ends with
    fatal: Remote branch <branch> not found in upstream origin.
  2. No test resource key. .github/workflows/pull-requests.yml passes no
    TestResourceKey to the shared workflow, so a cloud test either skips or
    fails with A resource key is required to access the cloud server.
  3. A failure in the examples repository that survives both of those.

Points 1 and 2 are fixed by
#180, which falls
back to main when the branch has no twin in the examples repository and passes
the bespoke resource key secret to the pull request and nightly jobs. Its own run
35252381918
shows both working, with the line
No 'fix/pr-workflow-examples-branch-and-key' branch in 'ip-intelligence-java-examples', using 'main'
and no resource key error anywhere in the log.

Point 3 is what that run then failed on, in the examples repository rather than
here:

[ERROR] Tests run: 1, Failures: 0, Errors: 1 <<< FAILURE!
  - in fiftyone.ipintelligence.examples.console.CompareOnPremTest
java.util.concurrent.ExecutionException: java.lang.RuntimeException: POLYGON ((...

So this pull request stays a draft until #180 has merged and the
CompareOnPremTest failure in ip-intelligence-java-examples has been cleared.
Both are outside this repository's control, so someone with the examples
repository and the CI secrets has to clear them.

Two cloud test classes were turned off in May 2025 with no reason given,
so ValueCloudTests and MissingPropertyHandlingTests have not run since,
six tests in all. Turning them back on showed two real problems.

The fixture in MissingPropertyHandlingTests still named the element
ip-intelligence, whereas the engine reads ip, so the test threw looking
for a key the response did not have.

The value type test asked every listed property for its value. A property
the key is not entitled to, or one the client script populates, is listed
but carries no value and gives a reason instead, and asking such a value
for its type throws. It now reads whether there is a value first, checks
that the reason was given when there is none, and checks the type when
there is one.

Checked against the live cloud with a paid key, ip-intelligence.cloud
reports 12 tests run, none failing and none skipped, where before the
change the six restored tests error.
@jwrosewell

Copy link
Copy Markdown
Contributor Author

Run on this branch: the cloud tests come back on, but nothing gives them a resource key

Run: https://github.com/51Degrees/ip-intelligence-java/actions/runs/35192147021

The workflow was dispatched against this branch to get real evidence in place of the link lint a draft gets. It built the branch on its own as pull request 0, so nothing was merged.

What passed

The on premise side is fine. The Ubuntu ARM Java 17 job reports "Tests run: 3, Failures: 0, Errors: 0, Skipped: 0", then "Tests run: 22, Failures: 0, Errors: 0, Skipped: 2" and "Tests run: 22, Failures: 0, Errors: 0, Skipped: 1", and the IP Intelligence, Shared, On Premise and Translation modules are all SUCCESS.

What failed, and why it is about this branch

The Cloud module fails, and the run ends with "Tests run: 12, Failures: 0, Errors: 4" and BUILD FAILURE. All four errors are the same, on ValueCloudTests:

fiftyone.pipeline.core.exceptions.PipelineConfigurationException:
A resource key is required to access the cloud server.
    at fiftyone.ipintelligence.cloud.data.ValueCloudTests.init(ValueCloudTests.java:36)

The four are ValueTests_Cloud_AvailableProperties, ValueTests_Cloud_RegisteredName, ValueTests_Cloud_ValueTypes and ValueTests_Cloud_TypedGetters. They error in setup, before reaching anything they mean to check.

The cause is in .github/workflows/pull-requests.yml. It passes only two secrets:

    secrets:
      token: ${{ secrets.ACCESS_TOKEN }}
      IpIntelligenceUrl: ${{ secrets.IPI_DATA_FILE_URL }}

There is no TestResourceKey, so the cloud tests have no key to use. For contrast, device-detection-java's own pull-requests.yml passes TestResourceKey: ${{ secrets.SUPER_RESOURCE_KEY }}, and pipeline-node passes TestResourceKey: ${{ secrets._51DEGREES_RESOURCE_KEY_BESPOKE }}.

So turning the cloud tests back on needs one more line in the workflow, giving TestResourceKey a resource key secret. The organisation has several that this repository can see, including _51DEGREES_RESOURCE_KEY_BESPOKE, _51DEGREES_RESOURCE_KEY_PAID, SUPER_RESOURCE_KEY and RESOURCE_KEY_CLOUD_V5_BESPOKE.

I have not picked one, because which key to use is an entitlement question rather than a formatting one. The key has to actually carry the IP intelligence properties these tests read, and a key that does not will come back saying the property is a paid feature, or will return no element data for the component at all, which reads like a broken service when it is the entitlement working correctly. Worth confirming which key carries them before wiring it in.

A second, separate failure in the same run

The integration step then failed for a different reason, the same one on the sibling branch that adds the dry run input:

Cloning 'ip-intelligence-java-examples'
fatal: Remote branch test/turn-the-cloud-tests-back-on not found in upstream origin

ci/run-integration-tests.ps1 clones the examples repository on the branch the run is on, which on a nightly is main and exists, and on a dispatch against a feature branch does not. That one is not about this branch's content and is written up on the pull request that adds the dry run input.

The pull request has not been marked ready and no review has been requested.

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.

1 participant