Skip to content

Commit

Permalink
Improved global state handling in tests related to requirement aggreg…
Browse files Browse the repository at this point in the history
…ation (#3363)

* Added the first failing test to demonstrate the issue

* Added more examples to demonstrate the issue

* Disabled the new examples temporarily to see if the CI build passes without them

* Ensure cache-related tests don't leave polluted state behind

* Re-enabled the new examples, expecting them to fail on CI

* Reset the ConfiguredEnvironment singleton to avoid state pollution

* Disabled the new examples to debug issues with singletons leaking state between tests

* Clean up the state of global singletons in WhenReadingRequirementsFromThePackageStructure

* Checking if it's the ConfiguredEnvironment that's leaking state

* Disabled the new examples as state leakage issues from other tests continue...

* Debugging possible state pollution by AggregateRequirements class

* Enabled an example that might cause state pollution

* Disabled the potentially polluting test

* Partially enabled the potentially leaking test

* Enabled the first part of the AggregateRequirements as it might be responsible for leaking state

* Debugging first argument

* Debugging second argument

* Fixed a typo

* disabled the potentially leaky argument instantiation

* Narrowed the leak down to SystemEnvironmentVariables; confirming

* Re-enabled instantiation of the second argument to see if that fails the build

* Debugging potential leaks at the AbstractRequirementsTagProvider level

* Debugging potential leakage in FileSystemRequirementsTagProvider constructor

* Removed the explicit cll to FileSystemRequirementsTagProvider as it will fail the build; leaving the individual parameter initialisation instead

* Narrowed the leak down to RequirementsConfiguration; confirming...

* Checking if clearing the DefaultCapabilityTypes cache resolves the state leakage

* Ensure all requirements-related tests properly clear their caches, so that they don't leak state

* Re-enabled examples that demonstrate issues with JavaScript requirement aggregation

* Disabled Serenity/JS-specific tests for now so that the other fixes can be merged
  • Loading branch information
jan-molak authored Dec 28, 2023
1 parent f4fd076 commit 21a0624
Show file tree
Hide file tree
Showing 9 changed files with 395 additions and 4 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,118 @@
package net.thucydides.model.requirements;

import net.serenitybdd.model.environment.ConfiguredEnvironment;
import net.thucydides.model.domain.RequirementCache;
import net.thucydides.model.requirements.model.Requirement;
import org.junit.jupiter.api.*;

import java.io.File;
import java.nio.file.Path;
import java.util.List;

import static org.assertj.core.api.AssertionsForInterfaceTypes.assertThat;

@Disabled("Examples that demonstrate how Serenity/JS requirements aggregation should work")
class AggregateRequirementsTest {

@BeforeEach
void setUp() {
ConfiguredEnvironment.reset();
DefaultCapabilityTypes.instance().clear();
RequirementCache.getInstance().clear();
}

@AfterAll
static void afterAll() {
ConfiguredEnvironment.reset();
DefaultCapabilityTypes.instance().clear();
RequirementCache.getInstance().clear();
}

@Nested
@DisplayName("when interpreting Serenity/JS test outcomes")
class SerenityJSTestOutcomes {

@Test
void should_treat_files_in_a_flat_directory_structure_as_representing_features() {

List<Requirement> requirements = requirementsFrom(pathTo("serenity-js/spec-0-levels"));

System.out.println(requirements);

assertThat(requirements).hasSize(1);

Requirement feature = requirements.get(0);

assertThat(feature.getChildren()).hasSize(0);

assertThat(feature.getName()).isEqualTo("card_payment");
assertThat(feature.getDisplayName()).isEqualTo("Card payment");
assertThat(feature.getType()).isEqualTo("feature");
}
}

@Test
void should_treat_files_in_a_single_level_directory_structure_as_representing_capabilities_and_features() {

List<Requirement> requirements = requirementsFrom(pathTo("serenity-js/spec-1-level"));

System.out.println(requirements);

assertThat(requirements).hasSize(1);

Requirement capability = requirements.get(0);
assertThat(capability.getName()).isEqualTo("payments");
assertThat(capability.getDisplayName()).isEqualTo("Payments");
assertThat(capability.getType()).isEqualTo("capability");

assertThat(capability.getChildren()).hasSize(1);

Requirement feature = capability.getChildren().get(0);
assertThat(feature.getName()).isEqualTo("card_payment");
assertThat(feature.getDisplayName()).isEqualTo("Card payment");
assertThat(feature.getType()).isEqualTo("feature");
}

@Test
void should_treat_files_in_a_two_level_directory_structure_as_representing_themes_capabilities_and_features() {

List<Requirement> requirements = requirementsFrom(pathTo("serenity-js/spec-2-levels"));

System.out.println(requirements);

assertThat(requirements).hasSize(1);

Requirement theme = requirements.get(0);
assertThat(theme.getName()).isEqualTo("ecommerce");
assertThat(theme.getDisplayName()).isEqualTo("Ecommerce");
assertThat(theme.getType()).isEqualTo("theme");

assertThat(theme.getChildren()).hasSize(1);

Requirement capability = theme.getChildren().get(0);
assertThat(capability.getName()).isEqualTo("payments");
assertThat(capability.getDisplayName()).isEqualTo("Payments");
assertThat(capability.getType()).isEqualTo("capability");

assertThat(capability.getChildren()).hasSize(1);

Requirement feature = capability.getChildren().get(0);
assertThat(feature.getName()).isEqualTo("card_payment");
assertThat(feature.getDisplayName()).isEqualTo("Card payment");
assertThat(feature.getType()).isEqualTo("feature");
}

private List<Requirement> requirementsFrom(Path exampleRootDirectory) {

Path requirementsDirectory = exampleRootDirectory.resolve("spec");
Path jsonOutcomesDirectory = exampleRootDirectory.resolve("outcomes");

Requirements requirements = new AggregateRequirements(jsonOutcomesDirectory, requirementsDirectory.toString());

return requirements.getRequirementsService().getRequirements();
}

private static Path pathTo(String resource) {
return new File(ClassLoader.getSystemClassLoader().getResource(resource).getFile()).toPath();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,7 @@
import net.thucydides.model.reports.TestOutcomeLoader;
import net.thucydides.model.requirements.model.Requirement;
import net.thucydides.model.util.EnvironmentVariables;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Nested;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.*;

import java.io.File;
import java.io.IOException;
Expand All @@ -31,6 +29,15 @@ class WhenReadingRequirementsFromACollectionOfTestOutcomes {

@BeforeEach
void clearCaches() {
ConfiguredEnvironment.reset();
DefaultCapabilityTypes.instance().clear();
RequirementCache.getInstance().clear();
}

@AfterAll
static void afterAll() {
ConfiguredEnvironment.reset();
DefaultCapabilityTypes.instance().clear();
RequirementCache.getInstance().clear();
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,10 +1,13 @@
package net.thucydides.model.requirements;

import net.serenitybdd.model.environment.ConfiguredEnvironment;
import net.thucydides.model.domain.RequirementCache;
import net.thucydides.model.environment.MockEnvironmentVariables;
import net.thucydides.model.requirements.model.Requirement;
import net.thucydides.model.util.EnvironmentVariables;
import net.thucydides.model.environment.MockEnvironmentVariables;
import org.junit.Test;
import org.junit.jupiter.api.AfterAll;
import org.junit.jupiter.api.BeforeEach;

import java.util.List;

Expand All @@ -13,6 +16,20 @@

public class WhenReadingRequirementsFromThePackageStructure {

@BeforeEach
void clearCaches() {
ConfiguredEnvironment.reset();
DefaultCapabilityTypes.instance().clear();
RequirementCache.getInstance().clear();
}

@AfterAll
static void afterAll() {
ConfiguredEnvironment.reset();
DefaultCapabilityTypes.instance().clear();
RequirementCache.getInstance().clear();
}

@Test
public void should_read_requirements_from_a_one_level_package_hierarchy() {

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
{
"id": "checkout-should-allow-the-customer-to-pick-their-preferred-card;macos-23-1-0;chromium-120-0-6099-28",
"name": "Checkout should allow the customer to pick their preferred card",
"title": "Checkout should allow the customer to pick their preferred card",
"manual": false,
"testSteps": [
{
"number": 1,
"description": "Tess starts with an empty basket",
"startTime": 1701531998839,
"children": [],
"reportData": [],
"screenshots": [],
"duration": 1897,
"result": "SUCCESS"
}
],
"userStory": {
"id": "card-payment",
"storyName": "Card payment",
"displayName": "Card payment",
"path": "card_payment",
"type": "feature",
"narrative": "",
"pathElements": [
{
"name": "card_payment",
"description": ""
}
]
},
"startTime": "2023-12-02T15:46:38.235Z",
"tags": [
{
"name": "card_payment",
"type": "feature",
"displayName": "Card payment"
},
{
"name": "macOS 23.1.0",
"type": "platform",
"platformName": "macOS",
"platformVersion": "23.1.0",
"displayName": "macOS 23.1.0"
},
{
"name": "chromium 120.0.6099.28",
"type": "browser",
"browserName": "chromium",
"browserVersion": "120.0.6099.28",
"displayName": "chromium 120.0.6099.28"
}
],
"featureTag": {
"name": "card_payment",
"type": "feature",
"displayName": "Card payment"
},
"testSource": "Serenity/JS",
"context": "mac,chrome",
"driver": "chromium",
"result": "SUCCESS",
"duration": 3499
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
import { describe, it } from '@serenity-js/playwright-test'

describe('Card payment', () => {

describe('Checkout', () => {
it('should allow the customer to pick their preferred card', async ({ actor }) => {
// ...
})
})
})
Original file line number Diff line number Diff line change
@@ -0,0 +1,73 @@
{
"id": "checkout-should-allow-the-customer-to-pick-their-preferred-card;macos-23-1-0;chromium-120-0-6099-28",
"name": "Checkout should allow the customer to pick their preferred card",
"title": "Checkout should allow the customer to pick their preferred card",
"manual": false,
"testSteps": [
{
"number": 1,
"description": "Tess starts with an empty basket",
"startTime": 1701531998839,
"children": [],
"reportData": [],
"screenshots": [],
"duration": 1897,
"result": "SUCCESS"
}
],
"userStory": {
"id": "card-payment",
"storyName": "Card payment",
"displayName": "Card payment",
"path": "payments/card_payment",
"type": "feature",
"narrative": "",
"pathElements": [
{
"name": "payments",
"description": ""
},
{
"name": "card_payment",
"description": ""
}
]
},
"startTime": "2023-12-02T15:46:38.235Z",
"tags": [
{
"name": "payments",
"type": "capability",
"displayName": "Payments"
},
{
"name": "payments/card_payment",
"type": "feature",
"displayName": "Card payment"
},
{
"name": "macOS 23.1.0",
"type": "platform",
"platformName": "macOS",
"platformVersion": "23.1.0",
"displayName": "macOS 23.1.0"
},
{
"name": "chromium 120.0.6099.28",
"type": "browser",
"browserName": "chromium",
"browserVersion": "120.0.6099.28",
"displayName": "chromium 120.0.6099.28"
}
],
"featureTag": {
"name": "payments/card_payment",
"type": "feature",
"displayName": "Card payment"
},
"testSource": "Serenity/JS",
"context": "mac,chrome",
"driver": "chromium",
"result": "SUCCESS",
"duration": 3499
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
import { describe, it } from '@serenity-js/playwright-test'

describe('Card payment', () => {

describe('Checkout', () => {
it('should allow the customer to pick their preferred card', async ({ actor }) => {
// ...
})
})
})
Loading

0 comments on commit 21a0624

Please sign in to comment.