Skip to content

Commit e8e87e3

Browse files
committed
stages fixes, metadata lifecycle and missing modules from report
1 parent 18a9c6b commit e8e87e3

9 files changed

Lines changed: 109 additions & 88 deletions

File tree

allure-hamcrest/build.gradle.kts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ dependencies {
1010
testImplementation("org.junit.jupiter:junit-jupiter-params")
1111
testImplementation("org.slf4j:slf4j-simple")
1212
testImplementation(project(":allure-assertj"))
13+
testImplementation(project(":allure-junit-platform"))
1314
testImplementation(project(":allure-java-commons-test"))
1415
testRuntimeOnly("org.junit.jupiter:junit-jupiter-engine")
1516
testRuntimeOnly("org.junit.platform:junit-platform-launcher")

allure-java-commons/src/main/java/io/qameta/allure/Allure.java

Lines changed: 5 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,6 @@
2222
import io.qameta.allure.model.Parameter;
2323
import io.qameta.allure.model.Status;
2424
import io.qameta.allure.model.StepResult;
25-
import io.qameta.allure.model.WithMetadata;
2625
import io.qameta.allure.util.ExceptionUtils;
2726

2827
import java.io.ByteArrayInputStream;
@@ -31,7 +30,6 @@
3130
import java.util.Objects;
3231
import java.util.concurrent.CompletableFuture;
3332
import java.util.concurrent.CompletionStage;
34-
import java.util.function.Consumer;
3533

3634
import static io.qameta.allure.util.ResultsUtils.EPIC_LABEL_NAME;
3735
import static io.qameta.allure.util.ResultsUtils.FEATURE_LABEL_NAME;
@@ -264,7 +262,7 @@ public static void suite(final String value) {
264262
*/
265263
public static void label(final String name, final String value) {
266264
final Label label = new Label().setName(name).setValue(value);
267-
updateMetadata(metadata -> metadata.getLabels().add(label));
265+
getLifecycle().updateTestMetadata(metadata -> metadata.getLabels().add(label));
268266
}
269267

270268
/**
@@ -324,7 +322,7 @@ public static <T> T parameter(final String name, final T value,
324322
public static <T> T parameter(final String name, final T value,
325323
final Boolean excluded, final Parameter.Mode mode) {
326324
final Parameter parameter = createParameter(name, value, excluded, mode);
327-
updateMetadata(metadata -> metadata.getParameters().add(parameter));
325+
getLifecycle().updateTestMetadata(metadata -> metadata.getParameters().add(parameter));
328326
return value;
329327
}
330328

@@ -381,7 +379,7 @@ public static void link(final String name, final String url) {
381379
*/
382380
public static void link(final String name, final String type, final String url) {
383381
final Link link = new Link().setName(name).setType(type).setUrl(url);
384-
updateMetadata(metadata -> metadata.getLinks().add(link));
382+
getLifecycle().updateTestMetadata(metadata -> metadata.getLinks().add(link));
385383
}
386384

387385
/**
@@ -392,7 +390,7 @@ public static void link(final String name, final String type, final String url)
392390
* @see #descriptionHtml(String)
393391
*/
394392
public static void description(final String description) {
395-
updateMetadata(metadata -> metadata.setDescription(description));
393+
getLifecycle().updateTestMetadata(metadata -> metadata.setDescription(description));
396394
}
397395

398396
/**
@@ -404,7 +402,7 @@ public static void description(final String description) {
404402
* @see #description(String)
405403
*/
406404
public static void descriptionHtml(final String descriptionHtml) {
407-
updateMetadata(metadata -> metadata.setDescriptionHtml(descriptionHtml));
405+
getLifecycle().updateTestMetadata(metadata -> metadata.setDescriptionHtml(descriptionHtml));
408406
}
409407

410408
/**
@@ -521,13 +519,6 @@ public static void addHttpExchange(final String name, final HttpExchange exchang
521519
);
522520
}
523521

524-
private static void updateMetadata(final Consumer<WithMetadata> update) {
525-
final AllureLifecycle lifecycle = getLifecycle();
526-
lifecycle.getCurrentRootKey()
527-
.flatMap(lifecycle::metadataTarget)
528-
.ifPresent(target -> lifecycle.updateMetadata(target, update));
529-
}
530-
531522
private static void addAttachmentAsStep(final String name, final String type,
532523
final byte[] body, final AttachmentOptions options) {
533524
addAttachmentAsStep(name, type, new ByteArrayInputStream(body), options);

allure-java-commons/src/main/java/io/qameta/allure/AllureLifecycle.java

Lines changed: 36 additions & 63 deletions
Original file line numberDiff line numberDiff line change
@@ -206,7 +206,6 @@ public void writeScope(final AllureExternalKey key) {
206206
}
207207
final TestResultContainer container;
208208
synchronized (scope) {
209-
normalizeScope(scope.result());
210209
container = toScopeContainer(scope.result());
211210
}
212211
notifier.beforeContainerWrite(container);
@@ -400,7 +399,6 @@ private void startFixture(final AllureExternalKey scopeKey, final AllureExternal
400399
return;
401400
}
402401
synchronized (scope) {
403-
normalizeScope(scope.result());
404402
scope.result().getFixtures().add(
405403
new ScopeFixtureResult()
406404
.setUuid(UUID.randomUUID().toString())
@@ -641,35 +639,24 @@ public void updateStep(final AllureExternalKey key, final Consumer<StepResult> u
641639
}
642640

643641
/**
644-
* Updates current step. Stages are transparent to this: the update targets the deepest non-stage step, so a
645-
* caller finishing its own step is not misdirected onto a stage the user opened inside it.
642+
* Updates the current running step. A stage cannot be updated: stages are addressed by nobody once started, so
643+
* when the current step is a stage this warns and does nothing — a caller finishing its own step must address
644+
* it by key.
646645
*
647646
* @param update the update function.
648647
*/
649648
public void updateStep(final Consumer<StepResult> update) {
650-
final Optional<AllureExternalKey> current = currentNonStageStep();
649+
final Optional<AllureExternalKey> current = threadContext.getCurrentStep();
651650
if (current.isEmpty()) {
652651
LOGGER.warn("Could not update step: no step running");
653652
return;
654653
}
655-
updateStep(current.get(), update);
656-
}
657-
658-
private Optional<AllureExternalKey> currentNonStageStep() {
659-
final AllureExternalKey root = threadContext.getRoot().orElse(null);
660-
for (final AllureExternalKey key : threadContext.getLocalKeys()) {
661-
if (Objects.equals(key, root)) {
662-
return Optional.empty();
663-
}
664-
final Object item = items.get(key);
665-
if (!(item instanceof StepItem)) {
666-
return Optional.empty();
667-
}
668-
if (!((StepItem) item).stage()) {
669-
return Optional.of(key);
670-
}
654+
final Object item = items.get(current.get());
655+
if (item instanceof StepItem && ((StepItem) item).stage()) {
656+
LOGGER.warn("Could not update step: the current step is a stage");
657+
return;
671658
}
672-
return Optional.empty();
659+
updateStep(current.get(), update);
673660
}
674661

675662
/**
@@ -1065,53 +1052,35 @@ private static void waitForFutures(final Set<CompletableFuture<?>> futures) {
10651052
// ── Metadata ─────────────────────────────────────────────────────────────────────────────
10661053

10671054
/**
1068-
* Resolves the metadata target for the given executable key.
1055+
* Applies a metadata update to the test-level target of the calling thread's root executable: in a test, the
1056+
* test itself; in a before fixture, the fixture's scope — so the metadata propagates to every test of that
1057+
* scope when it stops. Metadata written in an after fixture is dropped by design: its tests are already
1058+
* stopped. Takes no effect if no executable is running.
10691059
*
1070-
* <p>Pure key-to-key mapping with no thread state: a test resolves to itself; a before fixture resolves to its
1071-
* scope (so before-fixture metadata propagates to the scope's tests); an after fixture, or anything else,
1072-
* resolves to empty.</p>
1073-
*
1074-
* @param key the executable key
1075-
* @return the metadata target key, or empty if the executable cannot carry metadata
1076-
*/
1077-
public Optional<AllureExternalKey> metadataTarget(final AllureExternalKey key) {
1078-
Objects.requireNonNull(key, EXTERNAL_KEY);
1079-
final Object item = items.get(key);
1080-
if (item instanceof TestItem) {
1081-
return Optional.of(key);
1082-
}
1083-
if (item instanceof FixtureItem && ScopeFixtureType.BEFORE.equals(((FixtureItem) item).type())) {
1084-
return Optional.of(((FixtureItem) item).scopeKey());
1085-
}
1086-
return Optional.empty();
1087-
}
1088-
1089-
/**
1090-
* Applies a metadata update to the test or scope identified by the given key.
1091-
*
1092-
* <p>Generic, key-addressed, and thread-free: it dispatches on the stored item behind the key. Higher-level
1093-
* facades resolve the current target (see {@link #metadataTarget(AllureExternalKey)}) and pass the mutation
1094-
* here.</p>
1095-
*
1096-
* @param key the metadata target key (a test or scope)
10971060
* @param update the metadata update
10981061
*/
1099-
public void updateMetadata(final AllureExternalKey key, final Consumer<WithMetadata> update) {
1100-
Objects.requireNonNull(key, EXTERNAL_KEY);
1101-
final Object item = items.get(key);
1102-
if (item instanceof ScopeItem) {
1103-
final ScopeItem scope = (ScopeItem) item;
1104-
synchronized (scope) {
1105-
normalizeScope(scope.result());
1106-
update.accept(scope.result());
1107-
}
1062+
public void updateTestMetadata(final Consumer<WithMetadata> update) {
1063+
final Optional<AllureExternalKey> root = threadContext.getRoot();
1064+
if (root.isEmpty()) {
1065+
LOGGER.warn("Could not update test metadata: no test or fixture running");
11081066
return;
11091067
}
1068+
final Object item = items.get(root.get());
11101069
if (item instanceof TestItem) {
1111-
updateTest(key, update::accept);
1070+
updateTest(root.get(), update::accept);
11121071
return;
11131072
}
1114-
LOGGER.warn("Could not update metadata: item with key {} is not a test or scope", key);
1073+
if (item instanceof FixtureItem && ScopeFixtureType.BEFORE.equals(((FixtureItem) item).type())) {
1074+
final ScopeItem scope = getItem(((FixtureItem) item).scopeKey(), ScopeItem.class, "update test metadata");
1075+
if (Objects.nonNull(scope)) {
1076+
synchronized (scope) {
1077+
update.accept(scope.result());
1078+
// the consumer is the only code that can break the lists-are-mutable invariant
1079+
normalizeScope(scope.result());
1080+
}
1081+
}
1082+
}
1083+
// after-fixture metadata is dropped by design — the scope's tests are already stopped
11151084
}
11161085

11171086
// ── Thread group ─────────────────────────────────────────────────────────────────────────
@@ -1303,7 +1272,6 @@ private void addChild(final ScopeItem scope, final String childUuid) {
13031272
return;
13041273
}
13051274
synchronized (scope) {
1306-
normalizeScope(scope.result());
13071275
if (!scope.result().getTests().contains(childUuid)) {
13081276
scope.result().getTests().add(childUuid);
13091277
}
@@ -1316,7 +1284,6 @@ private void applyScopeMetadata(final TestItem item) {
13161284
if (found instanceof ScopeItem) {
13171285
final ScopeItem scope = (ScopeItem) found;
13181286
synchronized (scope) {
1319-
normalizeScope(scope.result());
13201287
mergeScopeMetadata(scope.result(), item.result());
13211288
}
13221289
}
@@ -1353,6 +1320,12 @@ private static TestResultContainer toScopeContainer(final ScopeResult scope) {
13531320
return container;
13541321
}
13551322

1323+
/**
1324+
* Re-establishes the internal invariant that all scope lists are non-null and mutable. The model guarantees it
1325+
* at construction and every internal mutation preserves it; the only code that can break it is the user-supplied
1326+
* metadata consumer, so this runs once right after that consumer — never defensively anywhere else. Copying also
1327+
* detaches any list alias the consumer may have retained.
1328+
*/
13561329
private static void normalizeScope(final ScopeResult scope) {
13571330
scope.setTests(mutableList(scope.getTests()));
13581331
scope.setFixtures(mutableList(scope.getFixtures()));

allure-java-commons/src/main/java/io/qameta/allure/aspects/StepsAspects.java

Lines changed: 21 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
package io.qameta.allure.aspects;
1717

1818
import io.qameta.allure.Allure;
19+
import io.qameta.allure.AllureExternalKey;
1920
import io.qameta.allure.AllureLifecycle;
2021
import io.qameta.allure.Step;
2122
import io.qameta.allure.model.Parameter;
@@ -29,7 +30,10 @@
2930
import org.aspectj.lang.annotation.Pointcut;
3031
import org.aspectj.lang.reflect.MethodSignature;
3132

33+
import java.util.Deque;
34+
import java.util.LinkedList;
3235
import java.util.List;
36+
import java.util.Objects;
3337

3438
import static io.qameta.allure.util.AspectUtils.getName;
3539
import static io.qameta.allure.util.AspectUtils.getParameters;
@@ -51,6 +55,14 @@ protected AllureLifecycle initialValue() {
5155
}
5256
};
5357

58+
/**
59+
* Keys of the steps this aspect has started on the current thread, most recent first. The aspect owns its keys
60+
* so the final status update addresses exactly its own step — never a stage the user opened inside the method.
61+
* Deliberately not inheritable: a child thread runs its own start/stop pairs.
62+
*/
63+
private static final ThreadLocal<Deque<AllureExternalKey>> CURRENT_STEPS =
64+
ThreadLocal.withInitial(LinkedList::new);
65+
5466
/**
5567
* Configures the step annotation.
5668
*/
@@ -89,7 +101,9 @@ public void stepStart(final JoinPoint joinPoint) {
89101
.setName(name)
90102
.setParameters(parameters);
91103

92-
getLifecycle().startStep(result);
104+
final AllureExternalKey key = AllureExternalKey.random(StepsAspects.class);
105+
CURRENT_STEPS.get().push(key);
106+
getLifecycle().startStep(key, result);
93107
}
94108

95109
/**
@@ -102,10 +116,12 @@ public void stepStart(final JoinPoint joinPoint) {
102116
throwing = "e"
103117
)
104118
public void stepFailed(final Throwable e) {
105-
if (getLifecycle().getCurrentExecutableKey().isEmpty()) {
119+
final AllureExternalKey key = CURRENT_STEPS.get().poll();
120+
if (Objects.isNull(key)) {
106121
return;
107122
}
108123
getLifecycle().updateStep(
124+
key,
109125
s -> s
110126
.setStatus(getStatus(e).orElse(Status.BROKEN))
111127
.setStatusDetails(getStatusDetails(e).orElse(null))
@@ -118,10 +134,11 @@ public void stepFailed(final Throwable e) {
118134
*/
119135
@AfterReturning(pointcut = "anyMethod() && withStepAnnotation()")
120136
public void stepStop() {
121-
if (getLifecycle().getCurrentExecutableKey().isEmpty()) {
137+
final AllureExternalKey key = CURRENT_STEPS.get().poll();
138+
if (Objects.isNull(key)) {
122139
return;
123140
}
124-
getLifecycle().updateStep(s -> s.setStatus(Status.PASSED));
141+
getLifecycle().updateStep(key, s -> s.setStatus(Status.PASSED));
125142
getLifecycle().stopStep();
126143
}
127144

allure-java-commons/src/test/java/io/qameta/allure/AllureLifecycleTest.java

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -76,9 +76,7 @@ public void setUp() {
7676
}
7777

7878
private void applyMetadata(final Consumer<WithMetadata> update) {
79-
lifecycle.getCurrentRootKey()
80-
.flatMap(lifecycle::metadataTarget)
81-
.ifPresent(target -> lifecycle.updateMetadata(target, update));
79+
lifecycle.updateTestMetadata(update);
8280
}
8381

8482
@Test

allure-java-commons/src/test/java/io/qameta/allure/AllureStageTest.java

Lines changed: 25 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -123,23 +123,44 @@ void shouldCloseStageWhenEnclosingStepIsStoppedByKey() {
123123
}
124124

125125
@Test
126-
void shouldNotMisdirectAmbientStepUpdatesOntoOpenStage() {
126+
void shouldSkipAmbientStepUpdateWhenStageIsCurrent() {
127127
final AllureResults results = runWithinTestContext(() -> {
128128
final AllureLifecycle lifecycle = Allure.getLifecycle();
129129
lifecycle.startStep(new StepResult().setName("outer step"));
130130
Allure.stage("open phase");
131-
// an aspect finishing "outer step" updates then stops ambiently — the stage must be transparent
131+
// stages cannot be updated: the ambient update warns and does nothing
132132
lifecycle.updateStep(step -> step.setStatus(Status.FAILED));
133133
lifecycle.stopStep();
134134
});
135135

136136
final StepResult outer = results.getTestResults().get(0).getSteps().get(0);
137137
assertThat(outer.getName()).isEqualTo("outer step");
138-
assertThat(outer.getStatus()).isEqualTo(Status.FAILED);
138+
assertThat(outer.getStatus()).isNull();
139139

140140
final StepResult phase = outer.getSteps().get(0);
141141
assertThat(phase.getName()).isEqualTo("open phase");
142-
// the stage was closed gracefully, not marked by the enclosing step's failure
142+
// the stage closed gracefully, untouched by the skipped ambient update
143+
assertThat(phase.getStatus()).isEqualTo(Status.PASSED);
144+
assertThat(phase.getStage()).isEqualTo(Stage.FINISHED);
145+
}
146+
147+
@Test
148+
void shouldUpdateOwnStepByKeyWhileStageIsOpen() {
149+
final AllureResults results = runWithinTestContext(() -> {
150+
final AllureLifecycle lifecycle = Allure.getLifecycle();
151+
final AllureExternalKey stepKey = AllureExternalKey.random(AllureStageTest.class);
152+
lifecycle.startStep(stepKey, new StepResult().setName("outer step"));
153+
Allure.stage("open phase");
154+
// a caller finishing its own step addresses it by key — stages cannot interfere
155+
lifecycle.updateStep(stepKey, step -> step.setStatus(Status.FAILED));
156+
lifecycle.stopStep(stepKey);
157+
});
158+
159+
final StepResult outer = results.getTestResults().get(0).getSteps().get(0);
160+
assertThat(outer.getName()).isEqualTo("outer step");
161+
assertThat(outer.getStatus()).isEqualTo(Status.FAILED);
162+
163+
final StepResult phase = outer.getSteps().get(0);
143164
assertThat(phase.getStatus()).isEqualTo(Status.PASSED);
144165
assertThat(phase.getStage()).isEqualTo(Stage.FINISHED);
145166
}

0 commit comments

Comments
 (0)