Merge pull request #790 from sebright/deprecate-Tags-setState

Deprecate Tags.setState, and throw an exception when it is called after getState.

2 files changed, 36 insertions, 13 deletions

diff --git a/api/src/test/java/io/opencensus/tags/NoopTagsTest.java b/api/src/test/java/io/opencensus/tags/NoopTagsTest.java
index 8434bd83..db07520e 100644
--- a/api/src/test/java/io/opencensus/tags/NoopTagsTest.java
+++ b/api/src/test/java/io/opencensus/tags/NoopTagsTest.java
@@ -50,19 +50,42 @@ public final class NoopTagsTest {
 
   @Test
   public void noopTagsComponent() {
-    assertThat(NoopTags.getNoopTagsComponent().getTagger()).isSameAs(NoopTags.getNoopTagger());
-    assertThat(NoopTags.getNoopTagsComponent().getTagPropagationComponent())
+    assertThat(NoopTags.newNoopTagsComponent().getTagger()).isSameAs(NoopTags.getNoopTagger());
+    assertThat(NoopTags.newNoopTagsComponent().getTagPropagationComponent())
         .isSameAs(NoopTags.getNoopTagPropagationComponent());
   }
 
   @Test
+  @SuppressWarnings("deprecation")
   public void noopTagsComponent_SetState_DisallowsNull() {
-    TagsComponent noopTagsComponent = NoopTags.getNoopTagsComponent();
+    TagsComponent noopTagsComponent = NoopTags.newNoopTagsComponent();
     thrown.expect(NullPointerException.class);
     noopTagsComponent.setState(null);
   }
 
   @Test
+  @SuppressWarnings("deprecation")
+  public void preventSettingStateAfterGettingState_DifferentState() {
+    TagsComponent noopTagsComponent = NoopTags.newNoopTagsComponent();
+    noopTagsComponent.setState(TaggingState.DISABLED);
+    noopTagsComponent.getState();
+    thrown.expect(IllegalStateException.class);
+    thrown.expectMessage("State was already read, cannot set state.");
+    noopTagsComponent.setState(TaggingState.ENABLED);
+  }
+
+  @Test
+  @SuppressWarnings("deprecation")
+  public void preventSettingStateAfterGettingState_SameState() {
+    TagsComponent noopTagsComponent = NoopTags.newNoopTagsComponent();
+    noopTagsComponent.setState(TaggingState.DISABLED);
+    noopTagsComponent.getState();
+    thrown.expect(IllegalStateException.class);
+    thrown.expectMessage("State was already read, cannot set state.");
+    noopTagsComponent.setState(TaggingState.DISABLED);
+  }
+
+  @Test
   public void noopTagger() {
     Tagger noopTagger = NoopTags.getNoopTagger();
     assertThat(noopTagger.empty()).isSameAs(NoopTags.getNoopTagContext());
diff --git a/api/src/test/java/io/opencensus/tags/TagsTest.java b/api/src/test/java/io/opencensus/tags/TagsTest.java
index e87407db..dee517b6 100644
--- a/api/src/test/java/io/opencensus/tags/TagsTest.java
+++ b/api/src/test/java/io/opencensus/tags/TagsTest.java
@@ -56,20 +56,20 @@ public class TagsTest {
         .isEqualTo("io.opencensus.tags.NoopTags$NoopTagsComponent");
   }
 
+  // There is only one test that modifies tagging state in the Tags class, since the state is
+  // global, and it could affect other tests. NoopTagsTest has more thorough tests for tagging
+  // state.
   @Test
-  public void getState() {
-    assertThat(Tags.getState()).isEqualTo(TaggingState.DISABLED);
-  }
-
-  @Test
-  public void setState_IgnoresInput() {
+  @SuppressWarnings("deprecation")
+  public void testState() {
+    // Test that setState ignores its input.
     Tags.setState(TaggingState.ENABLED);
     assertThat(Tags.getState()).isEqualTo(TaggingState.DISABLED);
-  }
 
-  @Test(expected = NullPointerException.class)
-  public void setState_DisallowsNull() {
-    Tags.setState(null);
+    // Test that setState cannot be called after getState.
+    thrown.expect(IllegalStateException.class);
+    thrown.expectMessage("State was already read, cannot set state.");
+    Tags.setState(TaggingState.ENABLED);
   }
 
   @Test