Add tests for Enso Cloud auth + simple API mock for `Enso_User` #8511

radeusgd · 2023-12-11T16:05:10Z

Pull Request Description

Closes Review and Test Enso User and Cloud Utils #8354
Extends simple-httpbin with a simple mock of the Cloud API (currently it checks the token and serves the /users endpoint).
Renames simple-httpbin to http-test-helper.

Important Notes

Checklist

Please ensure that the following checklist has been satisfied before submitting the PR:

The documentation has been updated, if necessary.
Screenshots/screencasts have been attached, if there are any visual changes. For interactive or animated visual changes, a screencast is preferred.
All code follows the
Scala,
Java,
and
Rust
style guides. In case you are using a language not listed above, follow the Rust style guide.
All code has been tested:
- Unit tests have been written where possible.
- If GUI codebase was changed, the GUI was tested when built using ./run ide build.

GregoryTravis · 2023-12-11T16:11:51Z

distribution/lib/Standard/Test/0.0.0-dev/src/Test_Environment.enso

@@ -12,4 +12,11 @@ polyglot java import org.enso.base.Environment_Utils
   `System.getenv` Java call remains unchanged.
 unsafe_with_environment_override : Text -> Text -> Any -> Any
 unsafe_with_environment_override key value ~action =
-    Environment_Utils.with_environment_variable_override key value (_->action)
+    ## This has to be done in Enso, not in Java, due to the bug: https://github.com/enso-org/enso/issues/7117


Maybe create Environment_Utils.enso for this? Unless it is temporary until that bug is fixed.

I don't understand why should I create this separate file? What would be the advantage over keeping the code here?

I removed the Java counterpart with_environment_variable_override as due to #7117 it is problematic.

But keeping this logic here seems to make most sense - the override is meant only for testing - it should never be used in actual production code.

Because the original implementation was in Environment_Utils.java, as if it was possibly meant to be used more than once. If not, then it doesn't matter.

I see. No that utils file was written specifically for this test tool and it shouldn't really be used elsewhere.

jdunkerley

As discussed we should have the tests in Tests as the functions are in Base.

JaroslavTulach · 2023-12-18T06:03:51Z

tools/simple-httpbin/src/main/java/org/enso/shttp/cloud_mock/UsersHandler.java

Shall the tool continue to be called simple? It is becoming more and more complicated.

Sure, we can rename it.

As requested, I renamed it to http-test-helper in 583cabf

JaroslavTulach · 2023-12-18T06:06:06Z

std-bits/base/src/main/java/org/enso/base/Environment_Utils.java

-        overrides.remove(name);
-      }
-    }
+  public static void setOverride(String name, String value) {


How does one prevent this to be used in production?

We cannot. Nor was this prevented in the earlier version.

We need some way to override environment variables in tests.

The functionality is hidden from the user. It does not do anything more than setting these env vars before launching Enso - so if the user wants to abuse the hidden feature, they are free to do so.

hubertp

SimpleHTTPBin... my precious ;(

radeusgd requested review from jdunkerley, GregoryTravis, 4e6 and JaroslavTulach as code owners December 11, 2023 16:05

radeusgd changed the title ~~add test for API url override~~ Add tests for Enso Cloud auth + simple API mock for Enso_User Dec 11, 2023

radeusgd self-assigned this Dec 11, 2023

radeusgd added the CI: No changelog needed Do not require a changelog entry for this PR. label Dec 11, 2023

radeusgd linked an issue Dec 11, 2023 that may be closed by this pull request

Review and Test Enso User and Cloud Utils #8354

Closed

9 tasks

GregoryTravis approved these changes Dec 11, 2023

View reviewed changes

radeusgd force-pushed the wip/radeusgd/8352-cloud-followup-1 branch from 77a6489 to 20966e9 Compare December 12, 2023 11:06

radeusgd requested review from hubertp and Akirathan as code owners December 12, 2023 11:06

radeusgd force-pushed the wip/radeusgd/8354-cloud-tests-2 branch from fac3165 to 89ba2d4 Compare December 12, 2023 11:07

radeusgd force-pushed the wip/radeusgd/8352-cloud-followup-1 branch from 20966e9 to 3d6856b Compare December 14, 2023 12:58

radeusgd force-pushed the wip/radeusgd/8354-cloud-tests-2 branch from 89ba2d4 to 17887d0 Compare December 14, 2023 13:01

jdunkerley approved these changes Dec 15, 2023

View reviewed changes

radeusgd force-pushed the wip/radeusgd/8352-cloud-followup-1 branch from 3d6856b to f951403 Compare December 15, 2023 10:07

radeusgd force-pushed the wip/radeusgd/8354-cloud-tests-2 branch 3 times, most recently from ee87d57 to ba2a8db Compare December 15, 2023 16:17

Base automatically changed from wip/radeusgd/8352-cloud-followup-1 to develop December 15, 2023 17:58

radeusgd added 9 commits December 16, 2023 14:44

add test for API url override

d28717b

try workaround for bug #7117 but it still doesn't work...

c40294e

more general workaround for bug #7117

2d20f6a

test creds

a736a95

use a safer conversion (will have a clearer error message if it fails)

850c518

preparing mock cloud access tests

c5c47c9

fix

c12b657

implement the mock cloud API - /users endpoint

2d4abe9

fix camelCase issue

fa0921b

radeusgd added 4 commits December 16, 2023 14:44

remove old way of overriding env var which is now deprecated

88c9b1f

CR: move cloud tests to Tests (Base)

aa6b6f6

fix after rebasing on top of wrapped map errors PR

257c92a

javafmt

baf0acc

radeusgd force-pushed the wip/radeusgd/8354-cloud-tests-2 branch from ba2a8db to baf0acc Compare December 16, 2023 13:44

JaroslavTulach reviewed Dec 18, 2023

View reviewed changes

radeusgd added 2 commits December 18, 2023 11:02

Merge branch 'develop' into wip/radeusgd/8354-cloud-tests-2

90e146e

Merge branch 'develop' into wip/radeusgd/8354-cloud-tests-2

0e3d640

enso-bot bot mentioned this pull request Dec 19, 2023

Unsupported specialization error related to Warnings #8552

Closed

radeusgd added 2 commits December 19, 2023 12:00

Merge branch 'develop' into wip/radeusgd/8354-cloud-tests-2

c2b0ae6

rename simple-httpbin to http-test-helper

583cabf

radeusgd requested a review from mwu-tow as a code owner December 19, 2023 11:13

radeusgd added the CI: Ready to merge This PR is eligible for automatic merge label Dec 19, 2023

mwu-tow approved these changes Dec 19, 2023

View reviewed changes

javafmt

e875e64

JaroslavTulach approved these changes Dec 19, 2023

View reviewed changes

prettier

5e67b52

hubertp approved these changes Dec 19, 2023

View reviewed changes

mergify bot merged commit 724f8d2 into develop Dec 19, 2023

mergify bot deleted the wip/radeusgd/8354-cloud-tests-2 branch December 19, 2023 17:41

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add tests for Enso Cloud auth + simple API mock for `Enso_User` #8511

Add tests for Enso Cloud auth + simple API mock for `Enso_User` #8511

radeusgd commented Dec 11, 2023 •

edited

Loading

GregoryTravis Dec 11, 2023

radeusgd Dec 11, 2023

radeusgd Dec 11, 2023

GregoryTravis Dec 11, 2023

radeusgd Dec 11, 2023

jdunkerley left a comment

JaroslavTulach Dec 18, 2023

radeusgd Dec 18, 2023

radeusgd Dec 19, 2023

JaroslavTulach Dec 18, 2023

radeusgd Dec 18, 2023

hubertp left a comment

Add tests for Enso Cloud auth + simple API mock for Enso_User #8511

Add tests for Enso Cloud auth + simple API mock for Enso_User #8511

Conversation

radeusgd commented Dec 11, 2023 • edited Loading

Pull Request Description

Important Notes

Checklist

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jdunkerley left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hubertp left a comment

Choose a reason for hiding this comment

Add tests for Enso Cloud auth + simple API mock for `Enso_User` #8511

Add tests for Enso Cloud auth + simple API mock for `Enso_User` #8511

radeusgd commented Dec 11, 2023 •

edited

Loading