Spec annotations in the right place, approximately

Author: justinabrahms

lib/src/test/java/dev/openfeature/javasdk/HookSpecTests.java

@@ -140,6 +140,7 @@ void emptyApiHooks() {
                 .build();
     }
 
+    @Specification(number="4.3.2", text="The before stage MUST run before flag resolution occurs. It accepts a hook context (required) and hook hints (optional) as parameters and returns either an evaluation context or nothing.")
     @Test void before_runs_ahead_of_evaluation() {
         OpenFeatureAPI api = OpenFeatureAPI.getInstance();
         api.setProvider(new AlwaysBrokenProvider());
@@ -180,7 +181,12 @@ void emptyApiHooks() {
         verify(h, times(0)).error(any(), any(), any());
     }
 
+
     @Specification(number="4.4.1", text="The API, Client and invocation MUST have a method for registering hooks which accepts flag evaluation options")
+    @Specification(number="4.3.5", text="The after stage MUST run after flag resolution occurs. It accepts a hook context (required), flag evaluation details (required) and hook hints (optional). It has no return value.")
+    @Specification(number="4.4.2", text="Hooks MUST be evaluated in the following order:  - before: API, Client, Invocation - after: Invocation, Client, API - error (if applicable): Invocation, Client, API - finally: Invocation, Client, API")
+    @Specification(number="4.3.6", text="The error hook MUST run when errors are encountered in the before stage, the after stage or during flag resolution. It accepts hook context (required), exception representing what went wrong (required), and hook hints (optional). It has no return value.")
+    @Specification(number="4.3.7", text="The finally hook MUST run after the before, after, and error stages. It accepts a hook context (required) and hook hints (optional). There is no return value.")
     @Test void hook_eval_order() {
         List<String> evalOrder = new ArrayList<String>();
         OpenFeatureAPI api = OpenFeatureAPI.getInstance();
@@ -281,6 +287,8 @@ public void finallyAfter(HookContext<Boolean> ctx, ImmutableMap<String, Object>
     }
 
 
+    @Specification(number="4.2.1", text="hook hints MUST be a structure supports definition of arbitrary properties, with keys of type string, and values of type boolean | string | number | datetime | structure..")
+    @Specification(number="4.5.2", text="hook hints MUST be passed to each hook.")
     @Test void hook_hints() {
         OpenFeatureAPI api = OpenFeatureAPI.getInstance();
         api.setProvider(new NoOpProvider());
@@ -316,6 +324,7 @@ public void finallyAfter(HookContext<Boolean> ctx, ImmutableMap<String, Object>
                 .build());
     }
 
+    @Specification(number="4.5.1", text="Flag evaluation options MAY contain hook hints, a map of data to be provided to hook invocations.")
     @Test void missing_hook_hints() {
         FlagEvaluationOptions feo = FlagEvaluationOptions.builder().build();
         assertNotNull(feo.getHookHints());
@@ -343,7 +352,8 @@ public void finallyAfter(HookContext<Boolean> ctx, ImmutableMap<String, Object>
         order.verify(hook).finallyAfter(any(),any());
     }
 
-    @Test void error_hooks() {
+    @Specification(number="4.4.5", text="If an error occurs in the before or after hooks, the error hooks MUST be invoked.")
+    @Test void error_hooks__before() {
         Hook hook = mock(Hook.class);
         doThrow(RuntimeException.class).when(hook).before(any(), any());
         OpenFeatureAPI api = OpenFeatureAPI.getInstance();
@@ -355,6 +365,19 @@ public void finallyAfter(HookContext<Boolean> ctx, ImmutableMap<String, Object>
         verify(hook, times(1)).error(any(), any(), any());
     }
 
+    @Specification(number="4.4.5", text="If an error occurs in the before or after hooks, the error hooks MUST be invoked.")
+    @Test void error_hooks__after() {
+        Hook hook = mock(Hook.class);
+        doThrow(RuntimeException.class).when(hook).after(any(), any(), any());
+        OpenFeatureAPI api = OpenFeatureAPI.getInstance();
+        api.setProvider(new NoOpProvider());
+        Client client = api.getClient();
+        client.getBooleanValue("key", false, new EvaluationContext(),
+                FlagEvaluationOptions.builder().hook(hook).build());
+        verify(hook, times(1)).after(any(), any(), any());
+        verify(hook, times(1)).error(any(), any(), any());
+    }
+
     @Test void multi_hooks_early_out__before() {
         Hook hook = mock(Hook.class);
         Hook hook2 = mock(Hook.class);
@@ -377,6 +400,8 @@ public void finallyAfter(HookContext<Boolean> ctx, ImmutableMap<String, Object>
         verify(hook2, times(1)).error(any(), any(), any());
     }
 
+    @Specification(number="4.1.4", text="The evaluation context MUST be mutable only within the before hook.")
+    @Specification(number="4.3.3", text="Any evaluation context returned from a before hook MUST be passed to subsequent before hooks (via HookContext).")
     @Test void beforeContextUpdated() {
         EvaluationContext ctx = new EvaluationContext();
         Hook hook = mock(Hook.class);
@@ -404,6 +429,8 @@ public void finallyAfter(HookContext<Boolean> ctx, ImmutableMap<String, Object>
         assertEquals(hc.getCtx(), ctx);
 
     }
+
+    @Specification(number="4.3.4", text="When before hooks have finished executing, any resulting evaluation context MUST be merged with the invocation evaluation context with the invocation evaluation context taking precedence in the case of any conflicts.")
     @Test void mergeHappensCorrectly() {
         EvaluationContext hookCtx = new EvaluationContext();
         hookCtx.addStringAttribute("test", "broken");
@@ -437,28 +464,21 @@ public void finallyAfter(HookContext<Boolean> ctx, ImmutableMap<String, Object>
 
 
     @Specification(number="4.1.2", text="The hook context SHOULD provide: access to the client metadata and the provider metadata fields.")
-    @Specification(number="4.1.4", text="The evaluation context MUST be mutable only within the before hook.")
-    @Specification(number="4.2.1", text="hook hints MUST be a structure supports definition of arbitrary properties, with keys of type string, and values of type boolean | string | number | datetime | structure..")
     @Specification(number="4.2.2.1", text="Condition: Hook hints MUST be immutable.")
     @Specification(number="4.2.2.2", text="Condition: The client metadata field in the hook context MUST be immutable.")
     @Specification(number="4.2.2.3", text="Condition: The provider metadata field in the hook context MUST be immutable.")
-    @Specification(number="4.3.1", text="Hooks MUST specify at least one stage.")
-    @Specification(number="4.3.2", text="The before stage MUST run before flag resolution occurs. It accepts a hook context (required) and hook hints (optional) as parameters and returns either an evaluation context or nothing.")
-    @Specification(number="4.3.3", text="Any evaluation context returned from a before hook MUST be passed to subsequent before hooks (via HookContext).")
-    @Specification(number="4.3.4", text="When before hooks have finished executing, any resulting evaluation context MUST be merged with the invocation evaluation context with the invocation evaluation context taking precedence in the case of any conflicts.")
-    @Specification(number="4.3.5", text="The after stage MUST run after flag resolution occurs. It accepts a hook context (required), flag evaluation details (required) and hook hints (optional). It has no return value.")
-    @Specification(number="4.3.6", text="The error hook MUST run when errors are encountered in the before stage, the after stage or during flag resolution. It accepts hook context (required), exception representing what went wrong (required), and hook hints (optional). It has no return value.")
-    @Specification(number="4.3.7", text="The finally hook MUST run after the before, after, and error stages. It accepts a hook context (required) and hook hints (optional). There is no return value.")
-    @Specification(number="4.3.8.1", text="Instead of finally, finallyAfter SHOULD be used.")
-    @Specification(number="4.4.2", text="Hooks MUST be evaluated in the following order:  - before: API, Client, Invocation - after: Invocation, Client, API - error (if applicable): Invocation, Client, API - finally: Invocation, Client, API")
+
+
     @Specification(number="4.4.3", text="If a finally hook abnormally terminates, evaluation MUST proceed, including the execution of any remaining finally hooks.")
     @Specification(number="4.4.4", text="If an error hook abnormally terminates, evaluation MUST proceed, including the execution of any remaining error hooks.")
-    @Specification(number="4.4.5", text="If an error occurs in the before or after hooks, the error hooks MUST be invoked.")
-    @Specification(number="4.5.1", text="Flag evaluation options MAY contain hook hints, a map of data to be provided to hook invocations.")
-    @Specification(number="4.5.2", text="hook hints MUST be passed to each hook.")
+
     @Specification(number="4.5.3", text="The hook MUST NOT alter the hook hints structure.")
     @Test @Disabled void todo() {}
 
+    @Specification(number="4.3.1", text="Hooks MUST specify at least one stage.")
+    @Test void default_methods_so_impossible() {}
+
+    @Specification(number="4.3.8.1", text="Instead of finally, finallyAfter SHOULD be used.")
     @SneakyThrows
     @Test void doesnt_use_finally() {
         try {