refactor(ZMSKVR-686): zmsapiautomation to zmsautomation module with ATAF integration#1892
Open
ThomasAFink wants to merge 200 commits intonextfrom
Conversation
This commit implements the ATAF (Test Automation Framework) integration for ZMS API automation tests. The new zmsautomation module is a copy of zmsapiautomation with ATAF/Cucumber integration, while the original zmsapiautomation module remains unchanged for CI compatibility. Phase 1: Project Setup - Copied zmsapiautomation to zmsautomation (both modules coexist) - Moved Flyway migrations from flyway/ to src/main/resources/db/migration/ - Created ATAF directory structure (ataf/data, ataf/hooks, ataf/runner, ataf/steps) - Created features directory structure (features/zmsapi, features/zmscitizenapi) - Updated pom.xml with Maven profiles: * standalone profile (default, active) - for existing JUnit tests * ataf profile - with Artifactory repository and ATAF dependencies - Configured Surefire plugin for both JUnit (standalone) and TestNG (ataf) - Added dependencies: Cucumber, TestNG, Flyway, MySQL connector Phase 2: Configuration Files - Created cucumber.properties with feature paths and glue code configuration - Created testautomation.properties with log level settings - Created log4j2-test.properties with console appender - Created testng.xml for TestNG test execution Phase 3: ATAF Integration Code - Created TestData.java for environment configuration (LOCAL, DEV) - Created DatabaseHook.java with @BeforeSuite Flyway migration setup - Created TestRunner.java extending BasicTestNGRunner - Created ZmsApiSteps.java with step definitions for ZMS API endpoints: * Status endpoint * Availability endpoint * Scope information endpoint * Error handling scenarios - Created CitizenApiSteps.java with step definitions for Citizen API: * Offices and services endpoint * Booking flow * Cancellation flow Phase 4: Feature Files - Converted StatusEndpointTest to features/zmsapi/status.feature - Converted OfficesAndServicesEndpointTest to features/zmscitizenapi/offices-and-services.feature - Created features/zmsapi/availability.feature - Created features/zmsapi/appointments.feature - Created features/zmscitizenapi/booking.feature Phase 5: Documentation - Created README.md with: * Prerequisites and project structure * Test execution instructions for both profiles * Environment variables documentation * Database setup information * Test tags and CI/CD notes Phase 6: Migration Examples - Created features/zmsapi/scopes.feature (example with Scenario Outline) - Created features/zmscitizenapi/cancellation.feature (example with multiple scenarios) - Created features/zmsapi/error-handling.feature (error scenario examples) - Created features/zmsapi/data-driven-example.feature (data-driven testing patterns) - Created features/MIGRATION_GUIDE.md with: * Before/after conversion examples * Common step definition patterns * Best practices and migration checklist Key Features: - Dual profile support: standalone (CI) and ataf (local development) - Automatic Flyway database migrations before test execution - Comprehensive step definitions for common API operations - Example feature files demonstrating various Cucumber patterns - Migration guide for converting additional JUnit tests The zmsapiautomation module remains unchanged and continues to run in GitHub Actions CI. The zmsautomation module is ready for local testing with the ATAF profile (requires Artifactory access). Both modules will coexist until Phase 7 when ATAF becomes open source and CI can be updated.
ThomasAFink
changed the title
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughEstablishes a comprehensive Cucumber-based test automation framework for ZMS APIs and UI components. Introduces Maven POM configuration with ATAF integration, Flyway database migrations, test infrastructure (runners, hooks, step definitions), numerous Cucumber feature files for API and UI testing, Docker/environment support for browser automation, and reorganizes package structures. Updates CI/CD workflow to placeholder status and modifies hostname references from keycloak.local to keycloak. Changes
Sequence Diagram(s)The changes do not warrant a sequence diagram. While the framework is comprehensive, it consists primarily of configuration, test infrastructure setup, and page object/step definition additions that are not characterized by complex multi-component sequential flows. The test execution is straightforward data-driven behavior rather than intricate interactions requiring visualization. Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Poem
✨ Finishing Touches🧪 Generate unit tests (beta)
|
Add test execution script for running ATAF/Cucumber tests on city laptop. The script handles complete test environment setup including database migrations, health checks, and test execution with the ATAF profile. Changes: - Created zmsautomation-test script adapted from zmsapiautomation-test - Updated paths to use zmsautomation module and new Flyway location - Configured to run tests with -Pataf Maven profile - Added Cucumber report cleanup and display - Updated README.md with script usage instructions The script provides the same functionality as the original but: - Uses zmsautomation/src/main/resources/db/migration for Flyway - Runs tests with -Pataf profile for ATAF/Cucumber execution - Supports Cucumber-specific Maven arguments (tags, features) - Displays Cucumber report locations (JSON and HTML) Usage: ./zmsautomation/zmsautomation-test ./zmsautomation/zmsautomation-test -Dcucumber.filter.tags="@smoke"
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 8
🤖 Fix all issues with AI agents
In `@zmsautomation/pom.xml`:
- Around line 37-40: Update the AssertJ dependency to mitigate CVE-2026-24400 by
setting the assertj-core artifact (artifactId: assertj-core, groupId:
org.assertj) to version 3.27.7 or later; change the ${assertj.version} Maven
property value to "3.27.7" (or a newer safe release) so the pom uses the patched
AssertJ release and rerun the build to ensure tests pass.
In
`@zmsautomation/src/main/resources/db/migration/V3__organisation_test_data.sql`:
- Around line 4-10: The INSERT seed contains trailing spaces in the
Organisationsname values for organisation rows with OrganisationsID 10 and 11 in
the organisation table; remove those trailing whitespace characters from the
'Organisationsname' entries (e.g., "Referat für Stadtplanung und Bauordnung "
and "Münchner Stadtentwässerung (MSE) ") so they are stored without trailing
spaces and won't break string comparisons or tests.
In `@zmsautomation/src/test/java/dto/zmscitizenapi/Office.java`:
- Around line 13-54: The Office DTO currently only supports object JSON for
Office and address, but the schema allows ["array","object","null"]; update
Office to accept both by adding `@JsonDeserialize`(using =
OfficeDeserializer.class) on the Office class (and similarly
`@JsonDeserialize`(using = AddressDeserializer.class) on the address field or
Address class) and implement OfficeDeserializer to detect if incoming token is
START_ARRAY then read positional elements into the appropriate fields (map
indices observed in production — e.g., index 7 -> id, index 8 -> name — and
populate showAlternativeLocations, parentId, etc., as applicable), otherwise
delegate to default object deserialization; ensure AddressDeserializer also
handles array/object/null forms. Alternatively, if you prefer an index-capturing
approach, implement `@JsonAnySetter` in Office to accept unmapped array indices
into a List<Object> and then translate those index positions into the typed
fields.
In `@zmsautomation/src/test/java/zms/ataf/hooks/DatabaseHook.java`:
- Around line 20-24: The Flyway cleanup call is unguarded and will DROP ALL
OBJECTS because cleanDisabled(false) then calling flyway.clean(); modify the
DatabaseHook logic to prevent accidental runs against non-test DBs by checking
the target database name or an explicit opt-in before calling flyway.clean():
obtain the MYSQL_DATABASE (or equivalent) value used by Flyway and verify it
matches an allowed test pattern (e.g., contains "test" or a configured regex) OR
require an environment flag like ALLOW_FLYWAY_CLEAN=true to be set; only if the
check passes invoke flyway.clean(), otherwise log a clear warning and skip clean
while still running flyway.migrate(); update the code around the
cleanDisabled(false)/flyway.clean() calls in DatabaseHook.java to implement this
guard.
In `@zmsautomation/src/test/java/zms/ataf/steps/CitizenApiSteps.java`:
- Around line 149-165: The as(Response, TypeReference) method is duplicated and
creates a new ObjectMapper per call; extract it into a shared utility (e.g.,
ResponseHelper) or move to BaseRestSteps, create a single reusable private
static final ObjectMapper (MAPPER) and implement a static generic as(Response,
TypeReference) that uses MAPPER, plus a helper to build the same truncated error
message (used in the RuntimeException) so both CitizenApiSteps.as and
ZmsApiSteps.as call the shared method instead of duplicating logic.
In `@zmsautomation/src/test/java/zms/ataf/steps/ZmsApiSteps.java`:
- Around line 35-40: The step method theZmsApiIsAvailableWithLogging currently
ignores its loggingEnabled parameter; either remove this variant or use the
parameter to configure logging before calling theZmsApiIsAvailable(). To
implement, convert the loggingEnabled string to a boolean and apply it to the
test logging configuration (e.g., call a setter like
TestConfig.setLoggingEnabled(boolean) or otherwise configure the logger via
TestConfig), then call theZmsApiIsAvailable(); alternatively delete the
theZmsApiIsAvailableWithLogging method and any Gherkin that uses it to avoid the
unused parameter.
In `@zmsautomation/src/test/resources/features/zmsapi/availability.feature`:
- Around line 16-18: The scenario currently only checks the status code; add an
explicit step after "Then the response status code should be 200" to assert the
returned slots are empty (e.g., add "And the response contains an empty list of
available slots" or "Then the available slots list is empty") so the expectation
in the title "returns empty" is validated; locate the scenario with "When I
request available appointments for scope 99999" and update it to include this
new assertion step.
In `@zmsautomation/zmsapiautomation-test`:
- Around line 118-124: The generated DROP list in drop_sql can be truncated
because GROUP_CONCAT has a small default max length; fix by increasing
GROUP_CONCAT max length before building `@tables` — e.g. add "SET SESSION
group_concat_max_len = 1000000;" (or other sufficiently large value) at the
start of the drop_sql string so the SELECT using GROUP_CONCAT(...) to populate
`@tables` won’t truncate, keeping the rest of the logic (PREPARE/EXECUTE stmt, SET
FOREIGN_KEY_CHECKS) unchanged.
🧹 Nitpick comments (23)
zmsautomation/src/test/resources/log4j2-test.properties (1)
5-5: Consider removing%Lfrom the pattern to avoid expensive location lookups.Line numbers force location calculation and can slow logging even in tests; keep it only if you really need it.
♻️ Suggested tweak
-appender.console.layout.pattern=[%T] %d{HH:mm:ss} %-5p %c{1}:%L - %m%n +appender.console.layout.pattern=[%T] %d{HH:mm:ss} %-5p %c{1} - %m%nzmsautomation/src/test/resources/features/zmscitizenapi/offices-and-services.feature (1)
1-13: Add a scenario to validate unpublished access control.
Right now you only cover the happy path. Consider adding a scenario to ensure unpublished offices/services are filtered or denied so the contract is protected.💡 Suggested scenario addition
+ Scenario: GET /offices-and-services/ hides unpublished entries + Given unpublished offices and services exist + When I request the offices and services endpoint + Then the response status code should be 200 + And the response should not include unpublished offices or servicesBased on learnings, the endpoints
/offices,/services,/services-by-office, and/offices-by-serviceshould implement the same unpublished access control as/offices-and-servicesto prevent internal appointments from being booked through these endpoints.zmsautomation/src/main/resources/db/migration/V4__behoerde_test_data.sql (1)
9-43: Minor whitespace inconsistencies in test data.Some entries have leading or trailing whitespace that could cause unexpected test behavior:
- Line 9:
'Servicestelle Kita-Beiträge 'has trailing space- Line 9:
'Landsberger Straße 486 81241 München 'has trailing space- Line 19:
' Landsberger Str. 486...'has leading space- Line 39:
' PLAN-HAIV-13'has leading spaceIf tests compare these strings exactly, they may fail unexpectedly.
🔧 Proposed fix to trim whitespace
-(9, 3, 1, 'Führerscheinstelle - Sonderschalter', '', '', 0), -(11, 5, 1, 'Servicestelle Kita-Beiträge ', 'Landsberger Straße 486 81241 München ', '', 0), +(9, 3, 1, 'Führerscheinstelle - Sonderschalter', '', '', 0), +(11, 5, 1, 'Servicestelle Kita-Beiträge', 'Landsberger Straße 486 81241 München', '', 0),-(36, 3, 1, 'Standesamt München-Pasing', ' Landsberger Str. 486, 81241 München', 'Frau Binder', 0), +(36, 3, 1, 'Standesamt München-Pasing', 'Landsberger Str. 486, 81241 München', 'Frau Binder', 0),-(94, 10, 1, ' PLAN-HAIV-13', '', '', 0), +(94, 10, 1, 'PLAN-HAIV-13', '', '', 0),zmsautomation/src/test/java/zms/ataf/data/TestData.java (1)
22-24: Consider environment variable support for DEV URLs.The LOCAL environment correctly uses environment variables with sensible defaults. For consistency and flexibility, the DEV environment could follow the same pattern, allowing configuration without code changes.
♻️ Proposed refactor for configurable DEV URLs
// City DEV environment (optional) - DEV.addSystem("ZMS-API", "https://zms-dev.muenchen.de/terminvereinbarung/api/2"); - DEV.addSystem("ZMS-Citizen-API", "https://zms-dev.muenchen.de/terminvereinbarung/api/citizen"); + String devBaseUri = System.getenv().getOrDefault("DEV_BASE_URI", + "https://zms-dev.muenchen.de/terminvereinbarung/api/2"); + String devCitizenUri = System.getenv().getOrDefault("DEV_CITIZEN_API_BASE_URI", + "https://zms-dev.muenchen.de/terminvereinbarung/api/citizen"); + + DEV.addSystem("ZMS-API", devBaseUri); + DEV.addSystem("ZMS-Citizen-API", devCitizenUri);zmsautomation/src/test/java/zms/ataf/hooks/DatabaseHook.java (1)
9-13: Add logging for database operations visibility.Adding log statements before clean/migrate operations would help with debugging test failures and provide visibility into which database is being modified.
📝 Proposed logging addition
package zms.ataf.hooks; import org.testng.annotations.BeforeSuite; import org.flywaydb.core.Flyway; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; public class DatabaseHook { + private static final Logger LOG = LoggerFactory.getLogger(DatabaseHook.class); `@BeforeSuite` public static void setupDatabase() { + String host = System.getenv().getOrDefault("MYSQL_HOST", "db"); + String port = System.getenv().getOrDefault("MYSQL_PORT", "3306"); + String database = System.getenv().getOrDefault("MYSQL_DATABASE", "zmsbo"); + String dbUrl = String.format("jdbc:mysql://%s:%s/%s", - System.getenv().getOrDefault("MYSQL_HOST", "db"), - System.getenv().getOrDefault("MYSQL_PORT", "3306"), - System.getenv().getOrDefault("MYSQL_DATABASE", "zmsbo")); + host, port, database); + + LOG.info("Setting up test database: {}", dbUrl);zmsautomation/pom.xml (1)
144-159: Scope Flyway/MySQL dependencies to tests in the ATAF profile.
These appear only used fromsrc/test/java, so leaving them compile-scope bloats the artifact and can leak transitive deps. Prefer<scope>test</scope>.Proposed change
<dependency> <groupId>org.flywaydb</groupId> <artifactId>flyway-core</artifactId> <version>${flyway.version}</version> + <scope>test</scope> </dependency> <dependency> <groupId>org.flywaydb</groupId> <artifactId>flyway-mysql</artifactId> <version>${flyway.version}</version> + <scope>test</scope> </dependency> <!-- MySQL connector --> <dependency> <groupId>com.mysql</groupId> <artifactId>mysql-connector-j</artifactId> <version>8.2.0</version> + <scope>test</scope> </dependency>zmsautomation/src/test/java/base/BaseTest.java (1)
54-61: Wire the configured timeout into RestAssured to avoid hung tests.
TestConfig.getRequestTimeout()exists but isn’t applied; network hangs can stall CI. Consider configuring connection/socket timeouts in RestAssured’s HTTP client config.Proposed change
import io.restassured.config.ObjectMapperConfig; import io.restassured.config.RestAssuredConfig; +import io.restassured.config.HttpClientConfig; @@ - RestAssuredConfig config = RestAssuredConfig.config() - .objectMapperConfig(new ObjectMapperConfig(ObjectMapperType.JACKSON_2)); + int timeoutMs = TestConfig.getRequestTimeout(); + RestAssuredConfig config = RestAssuredConfig.config() + .httpClient(HttpClientConfig.httpClientConfig() + .setParam("http.connection.timeout", timeoutMs) + .setParam("http.socket.timeout", timeoutMs)) + .objectMapperConfig(new ObjectMapperConfig(ObjectMapperType.JACKSON_2)); @@ - // Note: Timeout configuration is available via TestConfig.getRequestTimeout() - // but REST-assured 5.x timeout API may need to be configured differently - // For now, using default REST-assured timeoutszmsautomation/zmsapiautomation-test (1)
64-66: Make the target module/migration path configurable (currently hard-coded to zmsapiautomation).This script lives under
zmsautomationbut still points tozmsapiautomationfor Flyway and Maven. If the intent is to drive the new ATAF module, this will run the legacy module/migrations instead. Consider parameterizing the module and Flyway path (or relocating/renaming the script if it’s meant for the legacy module) to avoid accidental mis-runs.💡 Example: parameterize module & Flyway path
- FLYWAY_DIR="$PROJECT_ROOT/zmsapiautomation/flyway" + ZMS_TEST_MODULE="${ZMS_TEST_MODULE:-zmsapiautomation}" + FLYWAY_DIR="${FLYWAY_DIR:-$PROJECT_ROOT/$ZMS_TEST_MODULE/flyway}"- rm -rf "$PROJECT_ROOT/zmsapiautomation/target/surefire-reports" 2>/dev/null || true + rm -rf "$PROJECT_ROOT/$ZMS_TEST_MODULE/target/surefire-reports" 2>/dev/null || true- cd "$PROJECT_ROOT/zmsapiautomation" + cd "$PROJECT_ROOT/$ZMS_TEST_MODULE"Also applies to: 217-222
zmsautomation/src/test/resources/features/zmsapi/scopes.feature (1)
10-28: Avoid duplicate coverage for the same scope IDs.The single-scope scenario overlaps with the outline examples (141/99999). Consider keeping one form to keep the suite fast and reduce redundancy.
zmsautomation/src/test/java/helpers/zmsapi/StatusResponseBuilder.java (1)
22-27: Make the defaultgeneratedvalue deterministic for tests.Using
Instant.now()ties the builder to wall-clock time; any tests that assertgeneratedcan become flaky. Consider injecting aClock(defaulting toClock.systemUTC()) so callers can supply a fixed time when needed.♻️ Suggested refactor
import java.time.Instant; +import java.time.Clock; import java.time.ZoneOffset; import java.time.format.DateTimeFormatter; @@ public class StatusResponseBuilder { + private final Clock clock; @@ - public StatusResponseBuilder() { - // Set default values - this.generated = DateTimeFormatter.ISO_OFFSET_DATE_TIME - .format(Instant.now().atOffset(ZoneOffset.UTC)); - this.server = "test-server"; - } + public StatusResponseBuilder() { + this(Clock.systemUTC()); + } + + public StatusResponseBuilder(Clock clock) { + this.clock = clock; + this.generated = DateTimeFormatter.ISO_OFFSET_DATE_TIME + .format(Instant.now(clock).atOffset(ZoneOffset.UTC)); + this.server = "test-server"; + }As per coding guidelines, keep configurable data at high levels.
zmsautomation/src/test/resources/features/zmsapi/appointments.feature (1)
16-20: Consider removing the TODO block from the feature file.Feature files are easier to maintain when they only contain executable scenarios. Consider tracking these planned scenarios in an issue/backlog (or a dedicated
@wipfeature) instead of inline TODO comments.✂️ Suggested cleanup
- # TODO: Add more appointment scenarios as needed - # - Create appointment - # - Get appointment details - # - Update appointment - # - Cancel appointmentAs per coding guidelines, comments should explain intent and avoid obvious noise.
zmsautomation/src/test/java/dto/zmscitizenapi/collections/OfficesAndServicesResponse.java (1)
19-26: Consider initializing lists to empty collections.The List fields are uninitialized, which could result in null values if the JSON response omits these fields. While
@JsonIgnoreProperties(ignoreUnknown = true)handles unknown fields, missing fields will remain null.♻️ Defensive initialization
`@JsonProperty`("offices") - private List<Office> offices; + private List<Office> offices = new ArrayList<>(); `@JsonProperty`("services") - private List<Service> services; + private List<Service> services = new ArrayList<>(); `@JsonProperty`("relations") - private List<OfficeServiceRelation> relations; + private List<OfficeServiceRelation> relations = new ArrayList<>();zmsautomation/src/test/java/zmscitizenapi/OfficesAndServicesEndpointTest.java (1)
29-39: Catching genericExceptionmay mask real failures.The broad
catch (Exception e)will silently swallow unexpected errors (e.g., network issues, malformed JSON) and attempt the fallback path. Consider catching a more specific exception type.♻️ Catch specific Jackson deserialization exceptions
try { officesAndServices = response.as(OfficesAndServicesResponse.class); - } catch (Exception e) { + } catch (com.fasterxml.jackson.databind.JsonMappingException e) { // Fallback to wrapped response if unwrapped fails ApiResponse<OfficesAndServicesResponse> apiResponse = as(response, new com.fasterxml.jackson.core.type.TypeReference<ApiResponse<OfficesAndServicesResponse>>() {});zmsautomation/src/test/java/helpers/zmscitizenapi/OfficesAndServicesResponseBuilder.java (1)
59-65: Consider defensive copying inbuild()to prevent aliasing issues.The builder assigns its internal lists directly to the response object. If the builder is reused or the lists are modified after
build(), both the builder and the response will be affected.♻️ Return new list copies
public OfficesAndServicesResponse build() { OfficesAndServicesResponse response = new OfficesAndServicesResponse(); - response.setOffices(offices); - response.setServices(services); - response.setRelations(relations); + response.setOffices(new ArrayList<>(offices)); + response.setServices(new ArrayList<>(services)); + response.setRelations(new ArrayList<>(relations)); return response; }zmsautomation/src/test/java/zms/ataf/steps/CitizenApiSteps.java (6)
1-16: Imports look reasonable; minor cleanup opportunity.The static import on line 10 (
org.hamcrest.Matchers.*) is only partially used (justnotNullValue()andempty()in assertions), while AssertJ assertions are used elsewhere with fully qualified names. Consider choosing one assertion style consistently.
42-48: Extract repeated baseUri resolution to a helper method.The pattern
baseUri != null ? baseUri : TestConfig.getCitizenApiBaseUri()is repeated across multiple methods. Consider extracting to a private helper method for consistency and reduced duplication.♻️ Proposed helper method
+ private String getEffectiveBaseUri() { + return baseUri != null ? baseUri : TestConfig.getCitizenApiBaseUri(); + } + `@When`("I request the offices and services endpoint") public void iRequestTheOfficesAndServicesEndpoint() { response = given() - .baseUri(baseUri != null ? baseUri : TestConfig.getCitizenApiBaseUri()) + .baseUri(getEffectiveBaseUri()) .when() .get("/offices-and-services/"); }
50-60: Placeholder booking implementation will fail tests.The empty JSON body
"{}"will likely cause validation errors or unexpected behavior. The TODO indicates this needs implementation based on actual booking API structure.Would you like me to help generate the booking request structure based on the DTOs available in the project, or open an issue to track this implementation?
67-87: Consider using static import for AssertJ assertions.The fully qualified
org.assertj.core.api.Assertions.assertThat()calls are verbose. A static import would improve readability while the fallback deserialization pattern is well-designed.♻️ Add static import
Add to imports:
import static org.assertj.core.api.Assertions.assertThat;Then simplify assertions:
- org.assertj.core.api.Assertions.assertThat(officesAndServices).isNotNull(); + assertThat(officesAndServices).isNotNull();
97-114: State management needed for confirmation numbers.These placeholder methods set only
baseUribut don't store confirmation number state. When implemented, you'll need instance variables to hold the confirmation numbers used in subsequent steps.♻️ Add state variable for confirmation number
private Response response; private String baseUri; + private String confirmationNumber; `@Given`("I have a valid appointment confirmation number") public void iHaveAValidAppointmentConfirmationNumber() { - // TODO: This would typically set up test data or retrieve a valid confirmation number baseUri = TestConfig.getCitizenApiBaseUri(); + // TODO: Set confirmationNumber = <retrieve or create valid appointment> }
128-147: Assertions are too weak to verify actual behavior.The assertions
body(not(empty()))only verify the response has content, not that it contains the expected cancellation state or error details. This doesn't align with test quality guidelines (readable, one assert per test for specific behavior).♻️ Proposed stronger assertions
`@Then`("the appointment should be cancelled") public void theAppointmentShouldBeCancelled() { - response.then() - .body(not(empty())); - // TODO: Add specific assertions for cancellation confirmation + response.then() + .body("data.status", equalTo("cancelled")); + // Adjust based on actual API response structure } `@Then`("the response should indicate the appointment was not found") public void theResponseShouldIndicateAppointmentNotFound() { - response.then() - .body(not(empty())); - // TODO: Add specific assertions for not found error + response.then() + .statusCode(404) + .body("meta.error", equalTo(true)); }zmsautomation/src/test/java/zms/ataf/steps/ZmsApiSteps.java (3)
1-17: Unused import on line 8.
io.restassured.RestAssuredon line 8 is imported but not used directly — the static import on line 10 (static io.restassured.RestAssured.*) provides the needed methods.♻️ Remove unused import
import io.restassured.response.Response; -import io.restassured.RestAssured;
107-132: Consider adding GET and POST for completeness.The generic
iSendARequestmethod handles DELETE, PUT, and PATCH but omits GET and POST, which are handled by other specific methods. For a truly generic method, consider supporting all HTTP methods for maximum reusability in feature files.♻️ Add missing HTTP methods
switch (method.toUpperCase()) { + case "GET": + response = given() + .baseUri(baseUri) + .when() + .get(endpoint); + break; + case "POST": + response = given() + .baseUri(baseUri) + .contentType("application/json") + .when() + .post(endpoint); + break; case "DELETE":
152-162: Assertions don't verify actual error structure.Both
theResponseShouldContainAnErrorMessage()andtheResponseShouldContainValidationErrors()have identical implementations that only check the body is not empty. Consider asserting on actual error fields (e.g.,meta.error,errorsarray).♻️ Proposed stronger assertions
`@Then`("the response should contain an error message") public void theResponseShouldContainAnErrorMessage() { - response.then() - .body(not(empty())); + response.then() + .body("meta.error", equalTo(true)) + .body("meta.message", notNullValue()); } `@Then`("the response should contain validation errors") public void theResponseShouldContainValidationErrors() { - response.then() - .body(not(empty())); + response.then() + .body("meta.error", equalTo(true)) + .body("errors", not(empty())); }
Comment on lines
+37
to
+40
| <groupId>org.assertj</groupId> | ||
| <artifactId>assertj-core</artifactId> | ||
| <version>${assertj.version}</version> | ||
| <scope>test</scope> |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
GHSA-rqfh-9r24-8c9r AssertJ XXE vulnerability
💡 Result:
GHSA-rqfh-9r24-8c9r (alias CVE-2026-24400) is an XML External Entity (XXE) issue in AssertJ Core (org.assertj:assertj-core) caused by insecure XML parser configuration in org.assertj.core.util.xml.XmlStringPrettyFormatter (it builds a DocumentBuilderFactory without disabling DTDs/external entities). This can be reached when parsing untrusted XML via either isXmlEqualTo(CharSequence) or xmlPrettyFormat(String). If reachable, it can enable arbitrary file read, SSRF, and DoS (Billion Laughs). [1][3]
- Affected versions:
>= 1.4.0up to and including3.27.6(GitHub lists this as1.4.0–1.7.1,2.0.0–2.9.1,3.0.0–3.27.6). [1] - Fixed version: 3.27.7. [1][2]
- Mitigation (preferred order): replace
isXmlEqualTo(CharSequence)with XMLUnit (deprecated since AssertJ 3.18.0), or upgrade to 3.27.7, or avoid using these APIs with untrusted XML. [1]
Sources: GitHub Security Advisory [1], AssertJ release notes for 3.27.7 [2], NVD CVE record [3].
🌐 Web query:
AssertJ GHSA-rqfh-9r24-8c9r 3.27.6 patched version
💡 Result:
For GHSA-rqfh-9r24-8c9r (CVE-2026-24400) in org.assertj:assertj-core, version 3.27.6 is vulnerable; the patched (fixed) version is 3.27.7 (upgrade to ≥ 3.27.7). [1] [2]
Sources: OSV advisory lists Fixed: 3.27.7 and affected versions up to 3.27.6. [1] [2]
Upgrade AssertJ to 3.27.7 to fix XXE vulnerability GHSA-rqfh-9r24-8c9r.
CVE-2026-24400 affects AssertJ Core 3.27.6 and earlier, exposing tests that parse untrusted XML via isXmlEqualTo() or xmlPrettyFormat() to XXE attacks (arbitrary file read, SSRF, DoS). Upgrade to AssertJ 3.27.7 or later, or replace XML assertions with XMLUnit.
🤖 Prompt for AI Agents
In `@zmsautomation/pom.xml` around lines 37 - 40, Update the AssertJ dependency to
mitigate CVE-2026-24400 by setting the assertj-core artifact (artifactId:
assertj-core, groupId: org.assertj) to version 3.27.7 or later; change the
${assertj.version} Maven property value to "3.27.7" (or a newer safe release) so
the pom uses the patched AssertJ release and rerun the build to ensure tests
pass.
Comment on lines
+4
to
+10
| INSERT IGNORE INTO `organisation` (`OrganisationsID`, `InfoBezirkID`, `KundenID`, `Organisationsname`, `Anschrift`, `kioskpasswortschutz`) VALUES | ||
| (3, 14, 1, 'Kreisverwaltungsreferat', '', 1), | ||
| (5, 14, 1, 'Sozialreferat', '', 0), | ||
| (7, 14, 1, 'Referat für Bildung und Sport', '', 0), | ||
| (10, 14, 1, 'Referat für Stadtplanung und Bauordnung ', 'Blumenstraße 28b', 0), | ||
| (11, 14, 1, 'Münchner Stadtentwässerung (MSE) ', 'Friedenstraße 40', 0), | ||
| (14, 14, 1, 'Personal- und Organisationsreferat', 'Rosenheimer Straße 118, 81669 München', 0); |
Contributor
There was a problem hiding this comment.
Trim trailing spaces in seeded organisation names.
Line 8–9 include trailing spaces in Organisationsname, which can break string comparisons and fixtures.
🩹 Suggested fix
-(10, 14, 1, 'Referat für Stadtplanung und Bauordnung ', 'Blumenstraße 28b', 0),
-(11, 14, 1, 'Münchner Stadtentwässerung (MSE) ', 'Friedenstraße 40', 0),
+(10, 14, 1, 'Referat für Stadtplanung und Bauordnung', 'Blumenstraße 28b', 0),
+(11, 14, 1, 'Münchner Stadtentwässerung (MSE)', 'Friedenstraße 40', 0),📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| INSERT IGNORE INTO `organisation` (`OrganisationsID`, `InfoBezirkID`, `KundenID`, `Organisationsname`, `Anschrift`, `kioskpasswortschutz`) VALUES | |
| (3, 14, 1, 'Kreisverwaltungsreferat', '', 1), | |
| (5, 14, 1, 'Sozialreferat', '', 0), | |
| (7, 14, 1, 'Referat für Bildung und Sport', '', 0), | |
| (10, 14, 1, 'Referat für Stadtplanung und Bauordnung ', 'Blumenstraße 28b', 0), | |
| (11, 14, 1, 'Münchner Stadtentwässerung (MSE) ', 'Friedenstraße 40', 0), | |
| (14, 14, 1, 'Personal- und Organisationsreferat', 'Rosenheimer Straße 118, 81669 München', 0); | |
| INSERT IGNORE INTO `organisation` (`OrganisationsID`, `InfoBezirkID`, `KundenID`, `Organisationsname`, `Anschrift`, `kioskpasswortschutz`) VALUES | |
| (3, 14, 1, 'Kreisverwaltungsreferat', '', 1), | |
| (5, 14, 1, 'Sozialreferat', '', 0), | |
| (7, 14, 1, 'Referat für Bildung und Sport', '', 0), | |
| (10, 14, 1, 'Referat für Stadtplanung und Bauordnung', 'Blumenstraße 28b', 0), | |
| (11, 14, 1, 'Münchner Stadtentwässerung (MSE)', 'Friedenstraße 40', 0), | |
| (14, 14, 1, 'Personal- und Organisationsreferat', 'Rosenheimer Straße 118, 81669 München', 0); |
🤖 Prompt for AI Agents
In `@zmsautomation/src/main/resources/db/migration/V3__organisation_test_data.sql`
around lines 4 - 10, The INSERT seed contains trailing spaces in the
Organisationsname values for organisation rows with OrganisationsID 10 and 11 in
the organisation table; remove those trailing whitespace characters from the
'Organisationsname' entries (e.g., "Referat für Stadtplanung und Bauordnung "
and "Münchner Stadtentwässerung (MSE) ") so they are stored without trailing
spaces and won't break string comparisons or tests.
Comment on lines
+13
to
+54
| @JsonProperty("id") | ||
| private Integer id; | ||
|
|
||
| @JsonProperty("name") | ||
| private String name; | ||
|
|
||
| @JsonProperty("showAlternativeLocations") | ||
| private Boolean showAlternativeLocations; | ||
|
|
||
| @JsonProperty("address") | ||
| private Address address; | ||
|
|
||
| @JsonProperty("displayNameAlternatives") | ||
| private List<String> displayNameAlternatives; | ||
|
|
||
| @JsonProperty("organization") | ||
| private String organization; | ||
|
|
||
| @JsonProperty("organizationUnit") | ||
| private String organizationUnit; | ||
|
|
||
| @JsonProperty("slotTimeInMinutes") | ||
| private Integer slotTimeInMinutes; | ||
|
|
||
| @JsonProperty("version") | ||
| private Double version; | ||
|
|
||
| @JsonProperty("geo") | ||
| private GeoCoordinates geo; | ||
|
|
||
| @JsonProperty("parentId") | ||
| private Integer parentId; | ||
|
|
||
| @JsonProperty("scope") | ||
| private Object scope; // Complex nested structure - using Object for now | ||
|
|
||
| @JsonProperty("disabledByServices") | ||
| private List<Integer> disabledByServices; | ||
|
|
||
| @JsonProperty("slotsPerAppointment") | ||
| private String slotsPerAppointment; | ||
|
|
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Search for Office-related schema files, test fixtures, or API documentation
find . -type f \( -name "*.json" -o -name "*office*" -o -name "*schema*" \) | grep -iE "(office|schema|fixture)" | head -20Repository: it-at-m/eappointment
Length of output: 1412
🏁 Script executed:
# Look for Office model usage, deserialization, or any custom deserializers
rg -l "Office" --type java | head -10Repository: it-at-m/eappointment
Length of output: 847
🏁 Script executed:
# Check for any test data or example payloads
rg -i "office.*response\|api.*office\|office.*payload" -A 5 | head -50Repository: it-at-m/eappointment
Length of output: 46
🏁 Script executed:
# Examine if Address is defined as array or object elsewhere
rg "class Address" -A 10 --type javaRepository: it-at-m/eappointment
Length of output: 1923
🏁 Script executed:
# Look for JsonDeserialize annotations on Office or Address
rg "@JsonDeserialize|@JsonAnySetter" --type java -B 2 -A 2 | head -30Repository: it-at-m/eappointment
Length of output: 46
🏁 Script executed:
# Examine the OfficesAndServicesResponseBuilder to see what structure it builds
cat zmsautomation/src/test/java/helpers/zmscitizenapi/OfficesAndServicesResponseBuilder.javaRepository: it-at-m/eappointment
Length of output: 2184
🏁 Script executed:
# Look at test cases in OfficesAndServicesEndpointTest to see actual responses
head -100 zmsautomation/src/test/java/zmscitizenapi/OfficesAndServicesEndpointTest.javaRepository: it-at-m/eappointment
Length of output: 1916
🏁 Script executed:
# Search for actual Office object instantiation or mock data
rg "new Office\|\.withId\|\.withName\|\.withAddress" -A 5 --type java | head -60Repository: it-at-m/eappointment
Length of output: 46
🏁 Script executed:
# Look for any JSON fixtures containing "office" data
fd -e json | xargs rg -l -i "office" | head -10Repository: it-at-m/eappointment
Length of output: 718
🏁 Script executed:
cat zmsentities/schema/citizenapi/office.jsonRepository: it-at-m/eappointment
Length of output: 4371
Address property and Office structure must support array deserialization formats per schema.
The schema at zmsentities/schema/citizenapi/office.json defines both Office and address as potentially ["array", "object", "null"] types. The current model only handles object representation and will fail deserialization when the API returns array-based structures. Learnings confirm this occurs in production (ID and name fields at indices 7 and 8 respectively). Implement a custom @JsonDeserialize or use Jackson's array/collection handling to support both formats, or add @JsonAnySetter to capture unmapped array indices if only object format is intended.
🤖 Prompt for AI Agents
In `@zmsautomation/src/test/java/dto/zmscitizenapi/Office.java` around lines 13 -
54, The Office DTO currently only supports object JSON for Office and address,
but the schema allows ["array","object","null"]; update Office to accept both by
adding `@JsonDeserialize`(using = OfficeDeserializer.class) on the Office class
(and similarly `@JsonDeserialize`(using = AddressDeserializer.class) on the
address field or Address class) and implement OfficeDeserializer to detect if
incoming token is START_ARRAY then read positional elements into the appropriate
fields (map indices observed in production — e.g., index 7 -> id, index 8 ->
name — and populate showAlternativeLocations, parentId, etc., as applicable),
otherwise delegate to default object deserialization; ensure AddressDeserializer
also handles array/object/null forms. Alternatively, if you prefer an
index-capturing approach, implement `@JsonAnySetter` in Office to accept unmapped
array indices into a List<Object> and then translate those index positions into
the typed fields.
Comment on lines
+20
to
+24
| .cleanDisabled(false) | ||
| .load(); | ||
|
|
||
| flyway.clean(); | ||
| flyway.migrate(); |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -type f -name "DatabaseHook.java" | head -5Repository: it-at-m/eappointment
Length of output: 126
🏁 Script executed:
cat -n zmsautomation/src/test/java/zms/ataf/hooks/DatabaseHook.javaRepository: it-at-m/eappointment
Length of output: 1093
🌐 Web query:
Flyway 10 cleanDisabled configuration API behavior
💡 Result:
In Flyway 10, cleanDisabled is a safety switch that controls whether clean is allowed to run:
- Default is
true(i.e., clean is disabled unless you explicitly enable it). (documentation.red-gate.com) - If
cleanDisabledistrue, attempting to runflyway clean(CLI) orflyway.clean()(API) will be blocked (Flyway errors rather than wiping the schema). (documentation.red-gate.com) - To allow cleaning, you must set it to
false, e.g. via the Java API:
Flyway.configure().cleanDisabled(false).load()(documentation.red-gate.com)
or on the CLI for a one-off run:flyway -cleanDisabled=false clean. (documentation.red-gate.com)
Citations:
- 1: https://documentation.red-gate.com/fd/flyway-clean-disabled-setting-277578981.html?utm_source=openai
- 2: https://documentation.red-gate.com/fd/flyway-clean-disabled-setting-277578981.html?utm_source=openai
- 3: https://documentation.red-gate.com/fd/flyway-clean-disabled-setting-277578981.html?utm_source=openai
- 4: https://documentation.red-gate.com/fd/flyway-clean-disabled-setting-277578981.html?utm_source=openai
Add a safeguard to prevent accidental cleaning of non-test databases.
With cleanDisabled(false), the flyway.clean() call will DROP ALL OBJECTS in the target database. While appropriate for test hooks, this could be catastrophic if the MYSQL_DATABASE environment variable is misconfigured to point to a shared or production database.
Add a check to ensure the database name matches expected test patterns, or require explicit opt-in via an environment variable:
Safeguard example
+ String database = System.getenv().getOrDefault("MYSQL_DATABASE", "zmsbo");
+
+ // Safety check: only allow clean on known test databases
+ if (!database.equals("zmsbo") && !database.contains("test")) {
+ String allowClean = System.getenv().getOrDefault("FLYWAY_ALLOW_CLEAN", "false");
+ if (!"true".equals(allowClean)) {
+ throw new IllegalStateException(
+ "Refusing to clean database '" + database + "'. Set FLYWAY_ALLOW_CLEAN=true to override.");
+ }
+ }
+
flyway.clean();
flyway.migrate();🤖 Prompt for AI Agents
In `@zmsautomation/src/test/java/zms/ataf/hooks/DatabaseHook.java` around lines 20
- 24, The Flyway cleanup call is unguarded and will DROP ALL OBJECTS because
cleanDisabled(false) then calling flyway.clean(); modify the DatabaseHook logic
to prevent accidental runs against non-test DBs by checking the target database
name or an explicit opt-in before calling flyway.clean(): obtain the
MYSQL_DATABASE (or equivalent) value used by Flyway and verify it matches an
allowed test pattern (e.g., contains "test" or a configured regex) OR require an
environment flag like ALLOW_FLYWAY_CLEAN=true to be set; only if the check
passes invoke flyway.clean(), otherwise log a clear warning and skip clean while
still running flyway.migrate(); update the code around the
cleanDisabled(false)/flyway.clean() calls in DatabaseHook.java to implement this
guard.
Comment on lines
+149
to
+165
| private <T> T as(Response response, TypeReference<T> typeReference) { | ||
| try { | ||
| com.fasterxml.jackson.databind.ObjectMapper mapper = new com.fasterxml.jackson.databind.ObjectMapper(); | ||
| String responseBody = response.asString(); | ||
| return mapper.readValue(responseBody, typeReference); | ||
| } catch (Exception e) { | ||
| String responseBody = response.asString(); | ||
| String errorMsg = String.format( | ||
| "Failed to deserialize response. Status: %d, Response body (first 500 chars): %s", | ||
| response.getStatusCode(), | ||
| responseBody != null && responseBody.length() > 500 | ||
| ? responseBody.substring(0, 500) + "..." | ||
| : responseBody | ||
| ); | ||
| throw new RuntimeException(errorMsg, e); | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Extract shared as() helper to avoid duplication.
This method is duplicated in ZmsApiSteps.java. Additionally, creating a new ObjectMapper on each call is inefficient. Consider:
- Moving to a shared utility class (or
BaseRestStepsif appropriate) - Reusing a single
ObjectMapperinstance
♻️ Proposed shared utility
Create a shared helper class:
public final class ResponseHelper {
private static final ObjectMapper MAPPER = new ObjectMapper();
private ResponseHelper() {}
public static <T> T as(Response response, TypeReference<T> typeReference) {
try {
return MAPPER.readValue(response.asString(), typeReference);
} catch (Exception e) {
throw new RuntimeException(buildErrorMessage(response), e);
}
}
private static String buildErrorMessage(Response response) {
String body = response.asString();
return String.format(
"Failed to deserialize response. Status: %d, Body (first 500 chars): %s",
response.getStatusCode(),
body != null && body.length() > 500 ? body.substring(0, 500) + "..." : body
);
}
}🤖 Prompt for AI Agents
In `@zmsautomation/src/test/java/zms/ataf/steps/CitizenApiSteps.java` around lines
149 - 165, The as(Response, TypeReference) method is duplicated and creates a
new ObjectMapper per call; extract it into a shared utility (e.g.,
ResponseHelper) or move to BaseRestSteps, create a single reusable private
static final ObjectMapper (MAPPER) and implement a static generic as(Response,
TypeReference) that uses MAPPER, plus a helper to build the same truncated error
message (used in the RuntimeException) so both CitizenApiSteps.as and
ZmsApiSteps.as call the shared method instead of duplicating logic.
Comment on lines
+35
to
+40
| @Given("the ZMS API is available with logging {string}") | ||
| public void theZmsApiIsAvailableWithLogging(String loggingEnabled) { | ||
| baseUri = TestConfig.getBaseUri(); | ||
| // Logging configuration would be set here if needed | ||
| theZmsApiIsAvailable(); | ||
| } |
Contributor
There was a problem hiding this comment.
Unused parameter loggingEnabled.
The loggingEnabled parameter is ignored. Either implement the logging configuration using TestConfig.isLoggingEnabled() or remove this variant step.
🐛 Proposed implementation
`@Given`("the ZMS API is available with logging {string}")
public void theZmsApiIsAvailableWithLogging(String loggingEnabled) {
baseUri = TestConfig.getBaseUri();
- // Logging configuration would be set here if needed
- theZmsApiIsAvailable();
+ if (Boolean.parseBoolean(loggingEnabled)) {
+ RestAssured.enableLoggingOfRequestAndResponseIfValidationFails();
+ }
+ given()
+ .baseUri(baseUri)
+ .when()
+ .get("/status/")
+ .then()
+ .statusCode(200);
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| @Given("the ZMS API is available with logging {string}") | |
| public void theZmsApiIsAvailableWithLogging(String loggingEnabled) { | |
| baseUri = TestConfig.getBaseUri(); | |
| // Logging configuration would be set here if needed | |
| theZmsApiIsAvailable(); | |
| } | |
| `@Given`("the ZMS API is available with logging {string}") | |
| public void theZmsApiIsAvailableWithLogging(String loggingEnabled) { | |
| baseUri = TestConfig.getBaseUri(); | |
| if (Boolean.parseBoolean(loggingEnabled)) { | |
| RestAssured.enableLoggingOfRequestAndResponseIfValidationFails(); | |
| } | |
| theZmsApiIsAvailable(); | |
| } |
🤖 Prompt for AI Agents
In `@zmsautomation/src/test/java/zms/ataf/steps/ZmsApiSteps.java` around lines 35
- 40, The step method theZmsApiIsAvailableWithLogging currently ignores its
loggingEnabled parameter; either remove this variant or use the parameter to
configure logging before calling theZmsApiIsAvailable(). To implement, convert
the loggingEnabled string to a boolean and apply it to the test logging
configuration (e.g., call a setter like TestConfig.setLoggingEnabled(boolean) or
otherwise configure the logger via TestConfig), then call
theZmsApiIsAvailable(); alternatively delete the theZmsApiIsAvailableWithLogging
method and any Gherkin that uses it to avoid the unused parameter.
Comment on lines
+16
to
+18
| Scenario: Get availability for invalid scope returns empty | ||
| When I request available appointments for scope 99999 | ||
| Then the response status code should be 200 |
Contributor
There was a problem hiding this comment.
Add an explicit assertion for the “returns empty” expectation.
The scenario title says the result is empty, but it only checks the status code. Add a step that asserts no slots are returned.
✅ Suggested step
Scenario: Get availability for invalid scope returns empty
When I request available appointments for scope 99999
Then the response status code should be 200
+ And the response should contain no available slots📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| Scenario: Get availability for invalid scope returns empty | |
| When I request available appointments for scope 99999 | |
| Then the response status code should be 200 | |
| Scenario: Get availability for invalid scope returns empty | |
| When I request available appointments for scope 99999 | |
| Then the response status code should be 200 | |
| And the response should contain no available slots |
🤖 Prompt for AI Agents
In `@zmsautomation/src/test/resources/features/zmsapi/availability.feature` around
lines 16 - 18, The scenario currently only checks the status code; add an
explicit step after "Then the response status code should be 200" to assert the
returned slots are empty (e.g., add "And the response contains an empty list of
available slots" or "Then the available slots list is empty") so the expectation
in the title "returns empty" is validated; locate the scenario with "When I
request available appointments for scope 99999" and update it to include this
new assertion step.
Comment on lines
+118
to
+124
| local drop_sql="SET FOREIGN_KEY_CHECKS = 0; | ||
| SET @tables = (SELECT GROUP_CONCAT(CONCAT('\`', table_name, '\`')) FROM information_schema.tables WHERE table_schema = '$DB_NAME' AND table_type = 'BASE TABLE'); | ||
| SET @sql = IF(@tables IS NULL OR @tables = '', 'DO 0', CONCAT('DROP TABLE IF EXISTS ', @tables)); | ||
| PREPARE stmt FROM @sql; | ||
| EXECUTE stmt; | ||
| DEALLOCATE PREPARE stmt; | ||
| SET FOREIGN_KEY_CHECKS = 1;" |
Contributor
There was a problem hiding this comment.
Prevent truncated DROP list for larger schemas.
GROUP_CONCAT defaults to a small max length; on larger schemas the generated DROP list can truncate and leave tables behind.
🔧 Suggested fix
- local drop_sql="SET FOREIGN_KEY_CHECKS = 0;
+ local drop_sql="SET FOREIGN_KEY_CHECKS = 0;
+SET SESSION group_concat_max_len = 102400;
SET `@tables` = (SELECT GROUP_CONCAT(CONCAT('\`', table_name, '\`')) FROM information_schema.tables WHERE table_schema = '$DB_NAME' AND table_type = 'BASE TABLE');📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| local drop_sql="SET FOREIGN_KEY_CHECKS = 0; | |
| SET @tables = (SELECT GROUP_CONCAT(CONCAT('\`', table_name, '\`')) FROM information_schema.tables WHERE table_schema = '$DB_NAME' AND table_type = 'BASE TABLE'); | |
| SET @sql = IF(@tables IS NULL OR @tables = '', 'DO 0', CONCAT('DROP TABLE IF EXISTS ', @tables)); | |
| PREPARE stmt FROM @sql; | |
| EXECUTE stmt; | |
| DEALLOCATE PREPARE stmt; | |
| SET FOREIGN_KEY_CHECKS = 1;" | |
| local drop_sql="SET FOREIGN_KEY_CHECKS = 0; | |
| SET SESSION group_concat_max_len = 102400; | |
| SET `@tables` = (SELECT GROUP_CONCAT(CONCAT('\`', table_name, '\`')) FROM information_schema.tables WHERE table_schema = '$DB_NAME' AND table_type = 'BASE TABLE'); | |
| SET `@sql` = IF(`@tables` IS NULL OR `@tables` = '', 'DO 0', CONCAT('DROP TABLE IF EXISTS ', `@tables`)); | |
| PREPARE stmt FROM `@sql`; | |
| EXECUTE stmt; | |
| DEALLOCATE PREPARE stmt; | |
| SET FOREIGN_KEY_CHECKS = 1;" |
🤖 Prompt for AI Agents
In `@zmsautomation/zmsapiautomation-test` around lines 118 - 124, The generated
DROP list in drop_sql can be truncated because GROUP_CONCAT has a small default
max length; fix by increasing GROUP_CONCAT max length before building `@tables` —
e.g. add "SET SESSION group_concat_max_len = 1000000;" (or other sufficiently
large value) at the start of the drop_sql string so the SELECT using
GROUP_CONCAT(...) to populate `@tables` won’t truncate, keeping the rest of the
logic (PREPARE/EXECUTE stmt, SET FOREIGN_KEY_CHECKS) unchanged.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@zmsautomation/zmsautomation-test`:
- Around line 89-95: The shell functions exec_sql and exec_sql_root currently
pass credentials via --password which exposes them in the process list; change
them to set MYSQL_PWD in the environment for the invoked command (or use a MySQL
option file) instead of passing --password, e.g. export/inline MYSQL_PWD for the
mysql invocation in exec_sql and exec_sql_root, and apply the same change to the
mysqldump invocation so neither mysql nor mysqldump receives passwords as
command-line arguments.
🧹 Nitpick comments (10)
zmsautomation/zmsautomation-test (6)
2-2: Inconsistent indentation onsetdirective.The
set -euo pipefailline has extra leading whitespace inconsistent with the rest of the script.🧹 Proposed fix
#!/usr/bin/env bash - set -euo pipefail +set -euo pipefail
59-59: Port extraction may be redundant or incorrect.If
MYSQL_PORTis already a plain port number (e.g.,3306), the##*:expansion returns the original value—which works. However, the comment suggests it handlestcp://host:port, butMYSQL_HOSTdefaults todb, not extracted from this variable. Document the expected format or simplify if only numeric ports are expected.
169-169: Hardcoded version threshold is fragile.Deleting from
flyway_schema_history WHERE version > 1assumes the base schema is version 1. If the baseline version changes, this logic breaks silently. Consider using a named constant or Flyway'scleancommand for test resets.
200-211: Inconsistent indentation in health check blocks.The
if/elseblocks have extra leading whitespace that differs from the rest of the script.🧹 Proposed fix
info "Pinging citizenapi: $CITIZEN_API_BASE_URI/offices-and-services/" if curl -sf --max-time 10 "$CITIZEN_API_BASE_URI/offices-and-services/" >/dev/null; then - info "✓ citizenapi offices-and-services responding" - else - warn "✗ citizenapi offices-and-services NOT responding" - fi + info "✓ citizenapi offices-and-services responding" +else + warn "✗ citizenapi offices-and-services NOT responding" +fi info "Pinging zmsapi: $BASE_URI/status/" if curl -sf --max-time 10 "$BASE_URI/status/" >/dev/null; then - info "✓ zmsapi status responding" - else - warn "✗ zmsapi status NOT responding" - fi + info "✓ zmsapi status responding" +else + warn "✗ zmsapi status NOT responding" +fi
286-286: Inconsistent indentation on final exit.Extra leading whitespace before
exit "$TEST_EXIT".🧹 Proposed fix
- exit "$TEST_EXIT" +exit "$TEST_EXIT"
129-131: Consider order of trap handlers.The
ERRtrap attempts to restore the DB on failure, but theEXITtrap always runsclear_cachesandcleanup_data_folder. Ifrestore_dbfails, the EXIT trap still runs cleanup—which is correct. However, if tests fail (non-zero exit), the ERR trap triggers restore before the explicit restore in section 12. This could cause a double-restore. Consider either removing the ERR trap restore (relying on the explicit restore path) or skipping section 12 restore on failure.zmsautomation/pom.xml (3)
145-154: ATAF dependencies missing<scope>test</scope>.The
de.muenchen.ataf.java:coreandde.muenchen.ataf.java:restdependencies default to compile scope. If these are only needed for tests, add<scope>test</scope>to prevent them from being included in production artifacts.🔧 Proposed fix
<dependency> <groupId>de.muenchen.ataf.java</groupId> <artifactId>core</artifactId> <version>0.2</version> + <scope>test</scope> </dependency> <dependency> <groupId>de.muenchen.ataf.java</groupId> <artifactId>rest</artifactId> <version>0.2</version> + <scope>test</scope> </dependency>
176-191: Consider test scope for Flyway and MySQL connector.If Flyway and the MySQL connector are only used for test database setup, they should have
<scope>test</scope>. This prevents them from being bundled with production artifacts.🔧 Proposed fix
<dependency> <groupId>org.flywaydb</groupId> <artifactId>flyway-core</artifactId> <version>${flyway.version}</version> + <scope>test</scope> </dependency> <dependency> <groupId>org.flywaydb</groupId> <artifactId>flyway-mysql</artifactId> <version>${flyway.version}</version> + <scope>test</scope> </dependency> <!-- MySQL connector --> <dependency> <groupId>com.mysql</groupId> <artifactId>mysql-connector-j</artifactId> <version>8.2.0</version> + <scope>test</scope> </dependency>
112-121:updatePolicy=alwaysmay slow builds.Using
<updatePolicy>always</updatePolicy>for both releases and snapshots forces Maven to check for updates on every build. Considerdailyorinterval:Xfor releases to reduce network overhead during local development.zmsautomation/README.md (1)
97-105: Clarify default value source.The defaults mentioned here differ from those in the script. For example,
BASE_URIdefault here shows port8080, but the script (line 67) defaults to port80. Consider aligning the documentation with the actual script defaults or clarifying that these are example values rather than actual defaults.
Comment on lines
+89
to
+95
| exec_sql() { | ||
| mysql --ssl=0 -h "$DB_HOST" -P "$DB_PORT" -u "$DB_USER" --password="$DB_PASSWORD" "$DB_NAME" "$@" | ||
| } | ||
|
|
||
| exec_sql_root() { | ||
| mysql --ssl=0 -h "$DB_HOST" -P "$DB_PORT" -u root --password="${MYSQL_ROOT_PASSWORD:-root}" "$@" | ||
| } |
Contributor
There was a problem hiding this comment.
Passwords visible in process list via command-line arguments.
Using --password="$DB_PASSWORD" exposes credentials in ps output. Consider using environment variables or a MySQL option file instead.
🔐 Recommended alternative using MYSQL_PWD
exec_sql() {
- mysql --ssl=0 -h "$DB_HOST" -P "$DB_PORT" -u "$DB_USER" --password="$DB_PASSWORD" "$DB_NAME" "$@"
+ MYSQL_PWD="$DB_PASSWORD" mysql --ssl=0 -h "$DB_HOST" -P "$DB_PORT" -u "$DB_USER" "$DB_NAME" "$@"
}
exec_sql_root() {
- mysql --ssl=0 -h "$DB_HOST" -P "$DB_PORT" -u root --password="${MYSQL_ROOT_PASSWORD:-root}" "$@"
+ MYSQL_PWD="${MYSQL_ROOT_PASSWORD:-root}" mysql --ssl=0 -h "$DB_HOST" -P "$DB_PORT" -u root "$@"
}Apply the same pattern to mysqldump on line 100.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| exec_sql() { | |
| mysql --ssl=0 -h "$DB_HOST" -P "$DB_PORT" -u "$DB_USER" --password="$DB_PASSWORD" "$DB_NAME" "$@" | |
| } | |
| exec_sql_root() { | |
| mysql --ssl=0 -h "$DB_HOST" -P "$DB_PORT" -u root --password="${MYSQL_ROOT_PASSWORD:-root}" "$@" | |
| } | |
| exec_sql() { | |
| MYSQL_PWD="$DB_PASSWORD" mysql --ssl=0 -h "$DB_HOST" -P "$DB_PORT" -u "$DB_USER" "$DB_NAME" "$@" | |
| } | |
| exec_sql_root() { | |
| MYSQL_PWD="${MYSQL_ROOT_PASSWORD:-root}" mysql --ssl=0 -h "$DB_HOST" -P "$DB_PORT" -u root "$@" | |
| } |
🤖 Prompt for AI Agents
In `@zmsautomation/zmsautomation-test` around lines 89 - 95, The shell functions
exec_sql and exec_sql_root currently pass credentials via --password which
exposes them in the process list; change them to set MYSQL_PWD in the
environment for the invoked command (or use a MySQL option file) instead of
passing --password, e.g. export/inline MYSQL_PWD for the mysql invocation in
exec_sql and exec_sql_root, and apply the same change to the mysqldump
invocation so neither mysql nor mysqldump receives passwords as command-line
arguments.
e2ff22d to
bd35b2c
Compare
…d-ataf-compatibility-and-setup-first-test-case
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In
`@zmsdb/migrations/91770274084-restore-waiting-statistics-from-archive-table-since-2026-02-05.sql`:
- Around line 274-298: The average wait-time assignments (e.g.,
w.echte_zeit_ab_00_spontan ... w.echte_zeit_ab_23_spontan) currently use
COALESCE(p.<col>, 0) which masks “no data” as zero; change these to preserve
NULL instead of forcing 0—either assign p.echte_zeit_ab_XX_spontan directly or
use COALESCE(p.<col>, NULL) (effectively remove the COALESCE fallback), and
apply the same change for the corresponding termin and wegezeit average columns
so downstream consumers can distinguish “no data” (NULL) from an actual 0
average.
- Around line 1-16: The migration deletes rows via "DELETE FROM wartenrstatistik
WHERE datum >= `@start_date`;" without transactional safety and has a
filename/date mismatch with "@start_date := '2026-02-04'"; wrap the entire
migration (temp table creation/use/INSERTs and the DELETE/REBUILD steps that
reference tmp_ba_raw, tmp_ba_agg, tmp_pivot and `@start_date`) in an explicit
transaction (START TRANSACTION; ... COMMIT; with error handling to ROLLBACK on
failure) so the DELETE is atomic with the rebuild, and either update the
filename to reflect '2026-02-04' or change `@start_date` to match the filename
'2026-02-05' to remove the mismatch.
- Around line 24-34: The temporary table creation uses STR_TO_DATE on a TIME
column and a MariaDB-specific engine and also converts zero wait times to NULL;
update the SELECT for tmp_ba_raw to use HOUR(Timestamp) directly (referencing
the Timestamp column and HOUR(...) expression), change or remove the ENGINE=Aria
clause to match the codebase (use ENGINE=InnoDB or omit engine specification),
and preserve wartezeit zeros instead of converting them to NULL (adjust the CASE
that sets waited_minutes so it keeps 0 values or otherwise distinguishes missing
data from a 0-minute wait while still rounding as needed).
zmsdb/migrations/91770274084-restore-waiting-statistics-from-archive-table-since-2026-02-05.sql
Show resolved
Hide resolved
zmsdb/migrations/91770274084-restore-waiting-statistics-from-archive-table-since-2026-02-05.sql
Show resolved
Hide resolved
zmsdb/migrations/91770274084-restore-waiting-statistics-from-archive-table-since-2026-02-05.sql
Show resolved
Hide resolved
…igration Remove the defaultabholerstandort column from V5__standort_test_data.sql migration file. This column has been removed from the database schema and should not be included in the test data migration. Changes: - Removed defaultabholerstandort from INSERT column list - Removed corresponding values from all VALUES rows - Removed accidentally copied zmsapiautomation-test file
…test_data.sql - Removed ausgabeschaltername from column list in INSERT statement - Removed corresponding values from all INSERT rows - Column was causing schema mismatch errors
…test_data.sql Removed both defaultabholerstandort and ausgabeschaltername columns from the INSERT statement column list and their corresponding values from all data rows to match the updated schema.
…ndort_test_data.sql Removed wartezeitveroeffentlichen column along with defaultabholerstandort and ausgabeschaltername from the INSERT statement column list and their corresponding values from all data rows to match the updated schema.
Removed qtv_url column along with defaultabholerstandort, ausgabeschaltername, and wartezeitveroeffentlichen from the INSERT statement column list and their corresponding values from all data rows to match the updated schema.
… last row Fixed trailing semicolon issue in the last INSERT row that was causing SQL syntax error during Flyway migration.
- Updated workflow from zmsapiautomation to zmsautomation - Updated Flyway path to zmsautomation/src/main/resources/db/migration - Updated Maven cache key and working directories - Added placeholder job that always succeeds until ATAF is open source - Full test job is commented out with TODO to enable when ATAF is open source
…d of zmsapiautomation - Updated API Testing section to reference zmsautomation - Updated script names and paths from zmsapiautomation to zmsautomation - Updated description to mention Cucumber and ATAF framework - Added information about running Cucumber feature files
…rence zmsautomation - Updated Dockerfile comment to reference zmsautomation instead of zmsapiautomation - Updated zmsautomation README migration notes
The zmsapiautomation module has been replaced by zmsautomation which uses Cucumber and ATAF. The old module is no longer needed.
Removed outdated note about zmsapiautomation module since it has been deleted and replaced by zmsautomation.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@zmsautomation/README.md`:
- Around line 97-98: The two default URIs in zmsautomation/README.md are
inconsistent with the main README: update the `BASE_URI` and
`CITIZEN_API_BASE_URI` entries to match the main README by removing the explicit
:8080 port so they read `http://localhost/terminvereinbarung/api/2` and
`http://localhost/terminvereinbarung/api/citizen` respectively; ensure the
variable names `BASE_URI` and `CITIZEN_API_BASE_URI` are the ones edited so both
READMEs present the same defaults.
🧹 Nitpick comments (1)
.github/workflows/zmsautomation-tests.yaml (1)
25-310: Consider extracting commented workflow to a separate file.The 250+ lines of commented code, while documented with a TODO, could be better managed. Per Clean Code guidelines, commented-out code should generally be removed. Consider one of these alternatives:
- Move to a separate file like
.github/workflows/zmsautomation-tests.yaml.ataf-templatethat can be swapped in when ATAF is open-sourced- Document in the module README with the full workflow YAML for future reference
- Create a GitHub issue with the workflow content to track the ATAF integration work
This keeps the active workflow clean while preserving the implementation for future use.
Comment on lines
+97
to
+98
| - `BASE_URI` - Base URI for ZMS API (default: `http://localhost:8080/terminvereinbarung/api/2`) | ||
| - `CITIZEN_API_BASE_URI` - Base URI for Citizen API (default: `http://localhost:8080/terminvereinbarung/api/citizen`) |
Contributor
There was a problem hiding this comment.
Inconsistent default port in documentation.
The default URIs show port 8080 here, but the main README.md (lines 471-472) shows no port (implying port 80). Consider aligning these defaults for consistency.
Lines 97-98:
- `BASE_URI` - Base URI for ZMS API (default: `http://localhost:8080/terminvereinbarung/api/2`)
- `CITIZEN_API_BASE_URI` - Base URI for Citizen API (default: `http://localhost:8080/terminvereinbarung/api/citizen`)
Main README defaults:
- `BASE_URI`: `http://localhost/terminvereinbarung/api/2`
- `CITIZEN_API_BASE_URI`: `http://localhost/terminvereinbarung/api/citizen`
📝 Proposed fix to align defaults
### API Endpoints
-- `BASE_URI` - Base URI for ZMS API (default: `http://localhost:8080/terminvereinbarung/api/2`)
-- `CITIZEN_API_BASE_URI` - Base URI for Citizen API (default: `http://localhost:8080/terminvereinbarung/api/citizen`)
+- `BASE_URI` - Base URI for ZMS API (default: `http://localhost/terminvereinbarung/api/2`)
+- `CITIZEN_API_BASE_URI` - Base URI for Citizen API (default: `http://localhost/terminvereinbarung/api/citizen`)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| - `BASE_URI` - Base URI for ZMS API (default: `http://localhost:8080/terminvereinbarung/api/2`) | |
| - `CITIZEN_API_BASE_URI` - Base URI for Citizen API (default: `http://localhost:8080/terminvereinbarung/api/citizen`) | |
| - `BASE_URI` - Base URI for ZMS API (default: `http://localhost/terminvereinbarung/api/2`) | |
| - `CITIZEN_API_BASE_URI` - Base URI for Citizen API (default: `http://localhost/terminvereinbarung/api/citizen`) |
🤖 Prompt for AI Agents
In `@zmsautomation/README.md` around lines 97 - 98, The two default URIs in
zmsautomation/README.md are inconsistent with the main README: update the
`BASE_URI` and `CITIZEN_API_BASE_URI` entries to match the main README by
removing the explicit :8080 port so they read
`http://localhost/terminvereinbarung/api/2` and
`http://localhost/terminvereinbarung/api/citizen` respectively; ensure the
variable names `BASE_URI` and `CITIZEN_API_BASE_URI` are the ones edited so both
READMEs present the same defaults.
- Removed StatusEndpointTest.java (replaced by status.feature) - Removed OfficesAndServicesEndpointTest.java (replaced by offices-and-services.feature) - Updated pom.xml comments to reflect migration to Cucumber/ATAF - Tests now run exclusively via Cucumber feature files with ATAF framework - Use -Pataf profile to run tests (zmsautomation-test script does this automatically) - Applied code formatting fixes
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Fix all issues with AI agents
In `@zmsautomation/pom.xml`:
- Around line 82-88: The Surefire configuration in the POM is inconsistent with
the comment: it includes the pattern **/*Test.java while the comment says "No
JUnit tests"; update the configuration to match intent by either removing the
<includes> entry for **/*Test.java or explicitly skipping those tests (e.g., add
<skipTests>true</skipTests> or add an
<excludes><exclude>**/*Test.java</exclude></excludes>) in the Surefire plugin
configuration, or alternatively update the comment to accurately describe why
the include is present; refer to the Surefire configuration block
(useSystemClassLoader, includes) to locate and change the settings.
In `@zmsautomation/src/test/java/zms/ataf/steps/CitizenApiSteps.java`:
- Around line 116-126: In iSubmitACancellationRequest, replace the hard-coded
confirmation placeholder with the real confirmation number captured from the
booking step (e.g., confirmationNumber or
bookingResponse.get("confirmationNumber")) and ensure it is present
(throw/failed assertion if null/empty) before sending the request; additionally
fetch/inspect the booked appointment's status (appointmentStatus) and
appointmentTime (appointmentTime) from bookingResponse (or by GET
/appointments/{confirmationNumber}) and assert appointmentStatus == "CONFIRMED"
and appointmentTime is after Instant.now(ZoneOffset.UTC) before issuing the
cancellation, then submit the DELETE using the confirmation number (either as
path param /appointments/{confirmationNumber} or in the request body) and assign
the result to response.
- Around line 34-40: The step method iHaveSelectedAValidServiceAndLocation in
class CitizenApiSteps is a placeholder that only sets baseUri which can mislead
tests; either mark it explicitly pending by throwing
io.cucumber.java.PendingException (e.g., throw new PendingException()) or
implement proper test-data setup by populating the service and location test
variables (e.g., set serviceId and locationId or call a test fixture/setup
helper) in addition to baseUri; apply the same change to the other placeholder
step methods in CitizenApiSteps so scenarios are either clearly pending or use
real test data.
- Around line 50-60: The step method iSubmitABookingRequestWithValidData
currently sends an empty JSON body; mark it pending instead until a proper
booking DTO/payload exists by replacing the HTTP call in that method with
throwing a Cucumber pending exception (e.g., throw new PendingException("Booking
request payload not implemented")); ensure you import the appropriate
PendingException class (io.cucumber.java.PendingException or
cucumber.api.PendingException) and keep a short TODO comment referencing
creation of the booking request DTO so future work can locate this spot.
🧹 Nitpick comments (10)
zmsautomation/src/test/java/zms/ataf/hooks/DatabaseHook.java (1)
10-13: Consider extracting default values to named constants.The hardcoded defaults (
"db","3306","zmsbo") are repeated inline. Extracting them toprivate static finalconstants improves readability and makes future changes easier.Suggested refactor
public class DatabaseHook { + private static final String DEFAULT_HOST = "db"; + private static final String DEFAULT_PORT = "3306"; + private static final String DEFAULT_DATABASE = "zmsbo"; + private static final String DEFAULT_USER = "zmsbo"; + private static final String DEFAULT_PASSWORD = "zmsbo"; `@BeforeSuite` public static void setupDatabase() { String dbUrl = String.format("jdbc:mysql://%s:%s/%s", - System.getenv().getOrDefault("MYSQL_HOST", "db"), - System.getenv().getOrDefault("MYSQL_PORT", "3306"), - System.getenv().getOrDefault("MYSQL_DATABASE", "zmsbo")); + System.getenv().getOrDefault("MYSQL_HOST", DEFAULT_HOST), + System.getenv().getOrDefault("MYSQL_PORT", DEFAULT_PORT), + System.getenv().getOrDefault("MYSQL_DATABASE", DEFAULT_DATABASE)); Flyway flyway = Flyway.configure() .dataSource(dbUrl, - System.getenv().getOrDefault("MYSQL_USER", "zmsbo"), - System.getenv().getOrDefault("MYSQL_PASSWORD", "zmsbo")) + System.getenv().getOrDefault("MYSQL_USER", DEFAULT_USER), + System.getenv().getOrDefault("MYSQL_PASSWORD", DEFAULT_PASSWORD))As per coding guidelines: "Replace magic numbers with named constants" and "Use searchable names."
zmsautomation/src/test/java/zms/ataf/steps/ZmsApiSteps.java (5)
43-44: Extract repeatedbaseUriresolution to a helper method.The pattern
baseUri != null ? baseUri : TestConfig.getBaseUri()is duplicated 7 times throughout the class. This violates the DRY principle.♻️ Proposed refactor to extract helper method
Add a private helper method:
private String resolveBaseUri() { return baseUri != null ? baseUri : TestConfig.getBaseUri(); }Then replace all occurrences, e.g., line 44:
response = given() - .baseUri(baseUri != null ? baseUri : TestConfig.getBaseUri()) + .baseUri(resolveBaseUri()) .when()Also applies to: 51-52, 85-86, 100-101, 107-108, 135-136, 143-144
151-161: Duplicate implementations with weak assertions.
theResponseShouldContainAnErrorMessage()andtheResponseShouldContainValidationErrors()have identical implementations that only verify the body is not empty. This doesn't validate actual error structure or validation error content.Consider asserting on specific error response fields (e.g., error code, message field, or validation error array).
♻️ Proposed improvement
`@Then`("the response should contain an error message") public void theResponseShouldContainAnErrorMessage() { response.then() - .body(not(empty())); + .body("meta.error", equalTo(true)) + .body("meta.message", notNullValue()); } `@Then`("the response should contain validation errors") public void theResponseShouldContainValidationErrors() { response.then() - .body(not(empty())); + .body("errors", not(empty())); }Adjust the JSON paths to match your actual API error response structure.
106-131: Consider adding GET and POST toiSendARequestfor consistency.The step handles DELETE, PUT, and PATCH but not GET or POST. While other dedicated steps exist for these methods, having an incomplete generic step may confuse feature authors. Either extend the switch to cover all methods or document the intended usage in a comment.
♻️ Proposed extension
switch (method.toUpperCase()) { + case "GET": + response = given() + .baseUri(baseUri) + .when() + .get(endpoint); + break; + case "POST": + response = given() + .baseUri(baseUri) + .when() + .post(endpoint); + break; case "DELETE":
182-198: InstantiateObjectMapperonce as a class field.Creating a new
ObjectMapperon everyas()invocation is unnecessary overhead.ObjectMapperis thread-safe for read operations after configuration and can be reused.♻️ Proposed refactor
public class ZmsApiSteps extends BaseRestSteps { private Response response; private String baseUri; + private static final com.fasterxml.jackson.databind.ObjectMapper OBJECT_MAPPER = + new com.fasterxml.jackson.databind.ObjectMapper(); // ... other methods ... private <T> T as(Response response, TypeReference<T> typeReference) { try { - com.fasterxml.jackson.databind.ObjectMapper mapper = new com.fasterxml.jackson.databind.ObjectMapper(); String responseBody = response.asString(); - return mapper.readValue(responseBody, typeReference); + return OBJECT_MAPPER.readValue(responseBody, typeReference); } catch (Exception e) {
63-75: Consider using static import for AssertJ assertions.Fully qualified
org.assertj.core.api.Assertions.assertThat(...)calls reduce readability. Add a static import to align with the existing Hamcrest static imports.♻️ Proposed improvement
Add to imports:
import static org.assertj.core.api.Assertions.assertThat;Then simplify the method:
- org.assertj.core.api.Assertions.assertThat(apiResponse).isNotNull(); - org.assertj.core.api.Assertions.assertThat(apiResponse.getMeta()).isNotNull(); - org.assertj.core.api.Assertions.assertThat(apiResponse.getMeta().getError()).isFalse(); + assertThat(apiResponse).isNotNull(); + assertThat(apiResponse.getMeta()).isNotNull(); + assertThat(apiResponse.getMeta().getError()).isFalse(); StatusResponse statusData = apiResponse.getData(); - org.assertj.core.api.Assertions.assertThat(statusData).isNotNull(); - org.assertj.core.api.Assertions.assertThat(statusData.getVersion()).isNotNull(); + assertThat(statusData).isNotNull(); + assertThat(statusData.getVersion()).isNotNull();zmsautomation/pom.xml (1)
113-161: Consider adding<scope>test</scope>to ATAF profile dependencies.ATAF core/rest, Flyway, and MySQL connector dependencies lack explicit scope and default to
compile. If these are only needed for test execution, adding<scope>test</scope>prevents them from being packaged into the final artifact.♻️ Proposed scope additions
<dependency> <groupId>de.muenchen.ataf.java</groupId> <artifactId>core</artifactId> <version>0.2</version> + <scope>test</scope> </dependency> <dependency> <groupId>de.muenchen.ataf.java</groupId> <artifactId>rest</artifactId> <version>0.2</version> + <scope>test</scope> </dependency>Apply similarly to Flyway and MySQL connector if they're test-only.
zmsautomation/src/test/java/zms/ataf/steps/CitizenApiSteps.java (3)
67-87: Consider asserting non-empty collections, not just non-null.
Null checks allow empty lists to pass, which can hide regressions in test data or API behavior.♻️ Suggested tightening
org.assertj.core.api.Assertions.assertThat(officesAndServices).isNotNull(); -org.assertj.core.api.Assertions.assertThat(officesAndServices.getOffices()).isNotNull(); -org.assertj.core.api.Assertions.assertThat(officesAndServices.getServices()).isNotNull(); -org.assertj.core.api.Assertions.assertThat(officesAndServices.getRelations()).isNotNull(); +org.assertj.core.api.Assertions.assertThat(officesAndServices.getOffices()).isNotEmpty(); +org.assertj.core.api.Assertions.assertThat(officesAndServices.getServices()).isNotEmpty(); +org.assertj.core.api.Assertions.assertThat(officesAndServices.getRelations()).isNotEmpty();
89-95: Capture the confirmation number for later cancellation steps.
Right now it’s asserted but not stored, so subsequent steps can’t use it reliably.🧩 Suggested change
response.then() .body("confirmationNumber", notNullValue()); +String confirmationNumber = response.then().extract().path("confirmationNumber"); +org.assertj.core.api.Assertions.assertThat(confirmationNumber).isNotBlank(); +this.confirmationNumber = confirmationNumber;You’ll also need a field near the other members:
private String confirmationNumber;
128-133: Add explicit cancellation assertions, not just “body not empty.”
A non-empty body can still represent an error; assert the expected success field or status to avoid false positives.
Comment on lines
+34
to
+40
| @Given("I have selected a valid service and location") | ||
| public void iHaveSelectedAValidServiceAndLocation() { | ||
| // This step is a placeholder for test data setup | ||
| // In a real scenario, this would set up test data or select specific service/location | ||
| // For now, we'll assume the booking endpoint handles the validation | ||
| baseUri = TestConfig.getCitizenApiBaseUri(); | ||
| } |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Find and examine the CitizenApiSteps.java file
fd "CitizenApiSteps.java" -x cat -n {}Repository: it-at-m/eappointment
Length of output: 8306
Mark this placeholder step as pending (or implement data setup).
The step name promises a valid selection but only sets baseUri, which can make scenarios misleading and cause flaky failures later. Multiple similar placeholder steps exist in this class (lines 98–102, 104–108, 110–114).
Suggested change
`@Given`("I have selected a valid service and location")
public void iHaveSelectedAValidServiceAndLocation() {
// This step is a placeholder for test data setup
// In a real scenario, this would set up test data or select specific service/location
// For now, we'll assume the booking endpoint handles the validation
baseUri = TestConfig.getCitizenApiBaseUri();
+ throw new io.cucumber.java.PendingException("TODO: implement service/location setup");
}🤖 Prompt for AI Agents
In `@zmsautomation/src/test/java/zms/ataf/steps/CitizenApiSteps.java` around lines
34 - 40, The step method iHaveSelectedAValidServiceAndLocation in class
CitizenApiSteps is a placeholder that only sets baseUri which can mislead tests;
either mark it explicitly pending by throwing io.cucumber.java.PendingException
(e.g., throw new PendingException()) or implement proper test-data setup by
populating the service and location test variables (e.g., set serviceId and
locationId or call a test fixture/setup helper) in addition to baseUri; apply
the same change to the other placeholder step methods in CitizenApiSteps so
scenarios are either clearly pending or use real test data.
Comment on lines
+50
to
+60
| @When("I submit a booking request with valid data") | ||
| public void iSubmitABookingRequestWithValidData() { | ||
| // TODO: Implement booking request step | ||
| // This will need to be implemented based on the actual booking API structure | ||
| response = given() | ||
| .baseUri(baseUri != null ? baseUri : TestConfig.getCitizenApiBaseUri()) | ||
| .contentType("application/json") | ||
| .body("{}") // Placeholder - needs actual booking data | ||
| .when() | ||
| .post("/appointments/"); | ||
| } |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# First, let's locate and read the CitizenApiSteps.java file
fd "CitizenApiSteps.java" -type fRepository: it-at-m/eappointment
Length of output: 234
🏁 Script executed:
#!/bin/bash
# Check the structure and content around lines 50-60
if [ -f "zmsautomation/src/test/java/zms/ataf/steps/CitizenApiSteps.java" ]; then
wc -l "zmsautomation/src/test/java/zms/ataf/steps/CitizenApiSteps.java"
fiRepository: it-at-m/eappointment
Length of output: 131
🏁 Script executed:
#!/bin/bash
# Read the specific section mentioned in the review
if [ -f "zmsautomation/src/test/java/zms/ataf/steps/CitizenApiSteps.java" ]; then
sed -n '40,70p' "zmsautomation/src/test/java/zms/ataf/steps/CitizenApiSteps.java" | cat -n
fiRepository: it-at-m/eappointment
Length of output: 1537
🏁 Script executed:
#!/bin/bash
# Search for TestData classes or booking/appointment related data structures
fd -e java | xargs rg -l "TestData|BookingData|AppointmentData|bookingRequest" --type java | head -20Repository: it-at-m/eappointment
Length of output: 236
🏁 Script executed:
#!/bin/bash
# Look for TestConfig class and getCitizenApiBaseUri method
rg "getCitizenApiBaseUri|class TestConfig" --type java -A 3Repository: it-at-m/eappointment
Length of output: 4277
🏁 Script executed:
#!/bin/bash
# Find other booking/appointment related step methods in CitizenApiSteps
if [ -f "zmsautomation/src/test/java/zms/ataf/steps/CitizenApiSteps.java" ]; then
rg "@When|@Given|@Then" "zmsautomation/src/test/java/zms/ataf/steps/CitizenApiSteps.java" -A 2
fiRepository: it-at-m/eappointment
Length of output: 2428
🏁 Script executed:
#!/bin/bash
# Read the TestData class to understand what test data is available
if [ -f "zmsautomation/src/test/java/zms/ataf/data/TestData.java" ]; then
cat -n "zmsautomation/src/test/java/zms/ataf/data/TestData.java"
fiRepository: it-at-m/eappointment
Length of output: 1366
🏁 Script executed:
#!/bin/bash
# Check TestDataBuilder for booking/appointment related data
if [ -f "zmsautomation/src/test/java/helpers/TestDataBuilder.java" ]; then
cat -n "zmsautomation/src/test/java/helpers/TestDataBuilder.java"
fiRepository: it-at-m/eappointment
Length of output: 1955
🏁 Script executed:
#!/bin/bash
# Look for any models/DTOs related to booking or appointment requests
fd -e java | xargs rg "class.*Booking|class.*Appointment|class.*Request" | grep -i "booking\|appointment\|request" | head -20Repository: it-at-m/eappointment
Length of output: 46
🏁 Script executed:
#!/bin/bash
# Check if there are any booking/appointment request models or DTOs anywhere in the codebase
rg "Booking.*Request|Appointment.*Request" --type java -lRepository: it-at-m/eappointment
Length of output: 127
🏁 Script executed:
#!/bin/bash
# Look at the entire CitizenApiSteps.java to understand the pattern across all steps
cat -n "zmsautomation/src/test/java/zms/ataf/steps/CitizenApiSteps.java" | head -80Repository: it-at-m/eappointment
Length of output: 3858
🏁 Script executed:
#!/bin/bash
# Check the helpers/zmscitizenapi directory for any existing booking/appointment builders
fd "." "zmsautomation/src/test/java/helpers/zmscitizenapi" -type f 2>/dev/null || echo "Directory may not exist or be empty"Repository: it-at-m/eappointment
Length of output: 99
🏁 Script executed:
#!/bin/bash
# Look for any request DTOs in the dto package structure
fd -e java "dto" | xargs rg "class.*Request|interface.*Request" | grep -i booking -i appointmentRepository: it-at-m/eappointment
Length of output: 108
🏁 Script executed:
#!/bin/bash
# Check what DTOs exist in the dto package
fd -e java "dto" | head -30Repository: it-at-m/eappointment
Length of output: 46
🏁 Script executed:
#!/bin/bash
# Check the structure of dto directories
find . -path "*/dto/*" -name "*.java" 2>/dev/null | head -30Repository: it-at-m/eappointment
Length of output: 470
🏁 Script executed:
#!/bin/bash
# Look for all files in dto packages
ls -la zmsautomation/src/test/java/dto/ 2>/dev/null || ls -la zmsautomation/src/*/dto/ 2>/dev/null || find . -type d -name "dto" 2>/dev/nullRepository: it-at-m/eappointment
Length of output: 344
🏁 Script executed:
#!/bin/bash
# Check the entire CitizenApiSteps file to see all the placeholder/TODO patterns
grep -n "TODO\|Placeholder\|needs\|placeholder" "zmsautomation/src/test/java/zms/ataf/steps/CitizenApiSteps.java"Repository: it-at-m/eappointment
Length of output: 1012
🏁 Script executed:
#!/bin/bash
# Check the feature files to understand what the booking test scenario expects
fd "\.feature" | xargs rg "submit a booking request with valid data" -B 5 -A 5Repository: it-at-m/eappointment
Length of output: 1229
Mark this step pending until booking request payload is implemented.
The step currently sends an empty JSON object {} instead of valid booking data. This causes the test to either fail unexpectedly or pass without validating the actual booking behavior. No request DTO exists in the codebase yet to support this implementation.
Until proper booking request data is created (or discovered), use a pending exception to signal the incomplete step:
Recommended change
`@When`("I submit a booking request with valid data")
public void iSubmitABookingRequestWithValidData() {
- // TODO: Implement booking request step
- // This will need to be implemented based on the actual booking API structure
- response = given()
- .baseUri(baseUri != null ? baseUri : TestConfig.getCitizenApiBaseUri())
- .contentType("application/json")
- .body("{}") // Placeholder - needs actual booking data
- .when()
- .post("/appointments/");
+ throw new io.cucumber.java.PendingException("TODO: implement valid booking request payload");
}🤖 Prompt for AI Agents
In `@zmsautomation/src/test/java/zms/ataf/steps/CitizenApiSteps.java` around lines
50 - 60, The step method iSubmitABookingRequestWithValidData currently sends an
empty JSON body; mark it pending instead until a proper booking DTO/payload
exists by replacing the HTTP call in that method with throwing a Cucumber
pending exception (e.g., throw new PendingException("Booking request payload not
implemented")); ensure you import the appropriate PendingException class
(io.cucumber.java.PendingException or cucumber.api.PendingException) and keep a
short TODO comment referencing creation of the booking request DTO so future
work can locate this spot.
Comment on lines
+116
to
+126
| @When("I submit a cancellation request") | ||
| public void iSubmitACancellationRequest() { | ||
| // TODO: Implement cancellation request | ||
| // This will need to be implemented based on the actual cancellation API structure | ||
| response = given() | ||
| .baseUri(baseUri != null ? baseUri : TestConfig.getCitizenApiBaseUri()) | ||
| .contentType("application/json") | ||
| .body("{\"confirmationNumber\": \"placeholder\"}") // Placeholder - needs actual confirmation number | ||
| .when() | ||
| .delete("/appointments/"); | ||
| } |
Contributor
There was a problem hiding this comment.
Cancellation request uses a hard-coded placeholder confirmation number.
This won’t exercise real cancellation logic. Use the confirmation number captured from the booking step and ensure it’s present before issuing the request.
🧩 Suggested change
`@When`("I submit a cancellation request")
public void iSubmitACancellationRequest() {
// TODO: Implement cancellation request
// This will need to be implemented based on the actual cancellation API structure
+ if (confirmationNumber == null || confirmationNumber.isBlank()) {
+ throw new IllegalStateException("confirmationNumber not set");
+ }
response = given()
.baseUri(baseUri != null ? baseUri : TestConfig.getCitizenApiBaseUri())
.contentType("application/json")
- .body("{\"confirmationNumber\": \"placeholder\"}") // Placeholder - needs actual confirmation number
+ .body(java.util.Map.of("confirmationNumber", confirmationNumber))
.when()
.delete("/appointments/");
}Based on learnings: When testing appointment cancellation in zmscitizenapi, ensure the process is in confirmed status and use UTC timezone for timestamp comparisons. The canBeCancelled() validation requires both a future timestamp and confirmed status.
🤖 Prompt for AI Agents
In `@zmsautomation/src/test/java/zms/ataf/steps/CitizenApiSteps.java` around lines
116 - 126, In iSubmitACancellationRequest, replace the hard-coded confirmation
placeholder with the real confirmation number captured from the booking step
(e.g., confirmationNumber or bookingResponse.get("confirmationNumber")) and
ensure it is present (throw/failed assertion if null/empty) before sending the
request; additionally fetch/inspect the booked appointment's status
(appointmentStatus) and appointmentTime (appointmentTime) from bookingResponse
(or by GET /appointments/{confirmationNumber}) and assert appointmentStatus ==
"CONFIRMED" and appointmentTime is after Instant.now(ZoneOffset.UTC) before
issuing the cancellation, then submit the DELETE using the confirmation number
(either as path param /appointments/{confirmationNumber} or in the request body)
and assign the result to response.
…form - Replace cucumber-testng with cucumber-junit-platform-engine - Remove TestNG dependency and testng.xml configuration - Update TestRunner to use JUnit Platform @suite annotation - Add junit-platform-suite-api dependency - Update surefire plugin configuration for JUnit Platform
…tests - Add junit-platform-suite runtime dependency (required for suite execution) - Add @SelectClasspathResource annotation to explicitly select feature files - This fixes the 'TestEngine with ID junit-jupiter failed to discover tests' error
…or suite discovery - Add junit-platform-suite-engine dependency explicitly - Configure surefire to exclude base test classes - Set failIfNoTests=false to handle JUnit Jupiter discovery issues - This attempts to fix the 'junit-jupiter failed to discover tests' error
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@zmsautomation/src/test/java/zms/ataf/runner/TestRunner.java`:
- Around line 12-16: The TestRunner class currently calls TestData.init() in a
static block and lacks Cucumber glue config; move TestData.init() into a
Cucumber global hook class (e.g., create zms.ataf.hooks.GlobalSetupHook with a
public static method annotated with io.cucumber.java.BeforeAll that calls
TestData.init()), remove the static initializer from TestRunner, and configure
the TestRunner to set the glue property by importing and using
GLUE_PROPERTY_NAME so Cucumber discovers hooks; also replace or adapt
TestNG-specific DatabaseHook annotations (e.g., `@BeforeSuite`) to
Cucumber-compatible hooks (io.cucumber.java.Before or BeforeAll) or ensure
TestNG compatibility as appropriate.
🧹 Nitpick comments (1)
zmsautomation/pom.xml (1)
43-45: Centralize remaining hard‑coded dependency versions in properties.Versions for jackson-databind (Line 45), ATAF core/rest (Line 118 and Line 123), junit-platform-suite-api (Line 142), and mysql-connector-j (Line 160) are literals while others use properties. Moving them to
<properties>keeps upgrades consistent and centralized.♻️ Suggested refactor
<properties> <maven.compiler.source>21</maven.compiler.source> <maven.compiler.target>21</maven.compiler.target> <project.build.sourceEncoding>UTF-8</project.build.sourceEncoding> <junit.jupiter.version>6.0.1</junit.jupiter.version> <restassured.version>5.5.7</restassured.version> <assertj.version>3.27.6</assertj.version> <surefire.version>3.5.4</surefire.version> <cucumber.version>7.18.0</cucumber.version> <testng.version>7.8.0</testng.version> <flyway.version>10.0.1</flyway.version> + <jackson.version>2.20.1</jackson.version> + <junit.platform.version>1.10.2</junit.platform.version> + <mysql.version>8.2.0</mysql.version> + <ataf.version>0.2</ataf.version> </properties> ... - <version>2.20.1</version> + <version>${jackson.version}</version> ... - <version>0.2</version> + <version>${ataf.version}</version> ... - <version>0.2</version> + <version>${ataf.version}</version> ... - <version>1.10.2</version> + <version>${junit.platform.version}</version> ... - <version>8.2.0</version> + <version>${mysql.version}</version>As per coding guidelines, keep configurable data at high levels.
Also applies to: 116-124, 139-143, 157-160
Comment on lines
+12
to
+16
| @Suite | ||
| @IncludeEngines("cucumber") | ||
| public class TestRunner { | ||
| static { | ||
| TestData.init(); |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
io.cucumber.java.BeforeAll @BeforeAll cucumber-junit-platform-engine 7.18.0 global test initialization
💡 Result:
For global (once-per-test-run) initialization with cucumber-junit-platform-engine 7.18.0, don’t use JUnit Jupiter’s org.junit.jupiter.api.BeforeAll on the suite/runner class—those lifecycle callbacks generally won’t fire because the suite class is only configuration for the JUnit Platform Cucumber engine. Instead, use Cucumber’s global hooks:
1) Put a Cucumber global hook in glue code
package com.mycompany.cucumber;
import io.cucumber.java.BeforeAll;
import io.cucumber.java.AfterAll;
public class GlobalHooks {
`@BeforeAll`
public static void globalSetup() {
// runs once before any scenario
}
`@AfterAll`
public static void globalTearDown() {
// runs once after all scenarios
}
}Cucumber documents io.cucumber.java.BeforeAll / AfterAll as global hooks that run once before/after all scenarios. [1]
2) Make sure your glue package is configured (so the hook is discovered)
Example JUnit Platform suite:
import org.junit.platform.suite.api.ConfigurationParameter;
import org.junit.platform.suite.api.IncludeEngines;
import org.junit.platform.suite.api.SelectClasspathResource;
import org.junit.platform.suite.api.Suite;
import static io.cucumber.junit.platform.engine.Constants.GLUE_PROPERTY_NAME;
`@Suite`
`@IncludeEngines`("cucumber")
`@SelectClasspathResource`("features")
`@ConfigurationParameter`(key = GLUE_PROPERTY_NAME, value = "com.mycompany.cucumber")
public class CucumberIT {}This is the standard way to point the engine at your step defs/hooks package via GLUE_PROPERTY_NAME ("cucumber.glue"). [2][3]
Notes
- The
@BeforeAll/@AfterAllhook methods must bestatic. [1] - If your build runs tests in multiple JVM forks, “once” means once per JVM fork (not once for the entire CI job). (That’s a build-level concern, not a Cucumber hook behavior.)
Sources:
[1] Cucumber “Global hooks” (BeforeAll / AfterAll) documentation. [2] Example JUnit Platform suite configuration for Cucumber engine. [3] GLUE_PROPERTY_NAME constant (cucumber.glue) in Cucumber JUnit Platform Engine constants.
🏁 Script executed:
cat -n zmsautomation/src/test/java/zms/ataf/runner/TestRunner.javaRepository: it-at-m/eappointment
Length of output: 707
🏁 Script executed:
fd -t f -name "*Hook*" -o -name "*Bootstrap*" zmsautomation/src/test/java/Repository: it-at-m/eappointment
Length of output: 294
🏁 Script executed:
rg "TestData.init" --type java -A 3 -B 3Repository: it-at-m/eappointment
Length of output: 532
🏁 Script executed:
fd -t f -name "TestData.java" zmsautomation/Repository: it-at-m/eappointment
Length of output: 294
🏁 Script executed:
fd "TestData.java" zmsautomation/src/Repository: it-at-m/eappointment
Length of output: 119
🏁 Script executed:
fd "Hook" zmsautomation/src/test/java/Repository: it-at-m/eappointment
Length of output: 124
🏁 Script executed:
find zmsautomation/src/test/java -type f -name "*Hook*" -o -type f -name "*Setup*"Repository: it-at-m/eappointment
Length of output: 124
🏁 Script executed:
cat zmsautomation/src/test/java/zms/ataf/hooks/DatabaseHook.javaRepository: it-at-m/eappointment
Length of output: 911
🏁 Script executed:
cat zmsautomation/src/test/java/zms/ataf/data/TestData.javaRepository: it-at-m/eappointment
Length of output: 1184
🏁 Script executed:
rg "GLUE_PROPERTY_NAME|cucumber\.glue|ConfigurationParameter" zmsautomation/src/test/java/ -A 2 -B 2Repository: it-at-m/eappointment
Length of output: 46
🏁 Script executed:
find zmsautomation/src/test/java/zms/ataf -type f -name "*.java" | head -20Repository: it-at-m/eappointment
Length of output: 364
🏁 Script executed:
cat zmsautomation/src/test/java/zms/ataf/steps/ZmsApiSteps.java | head -30Repository: it-at-m/eappointment
Length of output: 855
Configure glue path and move initialization to Cucumber global hook.
The static initializer creates a hidden dependency: TestData.init() only runs when TestRunner is loaded; tests run through other entrypoints will skip setup and fail. Additionally, TestRunner does not configure the glue path, so Cucumber won't discover hooks at all.
Move TestData.init() to a Cucumber global hook and configure the glue path:
✏️ Update TestRunner with glue configuration
`@Suite`
`@IncludeEngines`("cucumber")
`@SelectClasspathResource`("features")
+@ConfigurationParameter(key = GLUE_PROPERTY_NAME, value = "zms.ataf.hooks:zms.ataf.steps")
public class TestRunner {
- static {
- TestData.init();
- }
}Add import: import static io.cucumber.junit.platform.engine.Constants.GLUE_PROPERTY_NAME;
✨ Create Cucumber global hook (new or update existing hooks package)
package zms.ataf.hooks;
import io.cucumber.java.BeforeAll;
import zms.ataf.data.TestData;
public class GlobalSetupHook {
`@BeforeAll`
public static void globalSetup() {
TestData.init();
}
}Note: Use io.cucumber.java.BeforeAll (Cucumber), not JUnit's @org.junit.jupiter.api.BeforeAll.
Also note: DatabaseHook currently uses @BeforeSuite (TestNG), which won't work with JUnit Platform/Cucumber engine. Consider replacing with a Cucumber @Before hook or ensuring TestNG compatibility.
🤖 Prompt for AI Agents
In `@zmsautomation/src/test/java/zms/ataf/runner/TestRunner.java` around lines 12
- 16, The TestRunner class currently calls TestData.init() in a static block and
lacks Cucumber glue config; move TestData.init() into a Cucumber global hook
class (e.g., create zms.ataf.hooks.GlobalSetupHook with a public static method
annotated with io.cucumber.java.BeforeAll that calls TestData.init()), remove
the static initializer from TestRunner, and configure the TestRunner to set the
glue property by importing and using GLUE_PROPERTY_NAME so Cucumber discovers
hooks; also replace or adapt TestNG-specific DatabaseHook annotations (e.g.,
`@BeforeSuite`) to Cucumber-compatible hooks (io.cucumber.java.Before or
BeforeAll) or ensure TestNG compatibility as appropriate.
d3a92a5 to
6f0a7a0
Compare
…etup - Replace JUnit Platform Suite with TestNG (like zms-test-automation) - Use BasicTestNGRunner from ATAF (same as zms-test-automation) - Replace cucumber-junit-platform-engine with cucumber-testng - Add surefire-testng dependency to use TestNG provider - Remove JUnit Platform Suite dependencies - This should fix the 'junit-jupiter failed to discover tests' error
Member
Author
|
Increase navigation timeouts for the statistics start page and treat slow Keycloak redirects as non-fatal so the zmsstatistic UI scenarios are less sensitive to transient SSO and environment delays.
Member
Author
|
Increase navigation and window switch timeout for statistics context to 60s to better tolerate slow loads.
Member
Author
|
Contributor
|
Tip For best results, initiate chat on the files or code changes.
Great progress — Kundenstatistik passes end‑to‑end now. The remaining failure is isolated to Dienstleistungsstatistik and consistently happens while clicking the location submit (“Auswahl bestätigen”/Weiter). The stack shows a 30s TimeoutException on clicking the submit button. This is almost always caused by one of:
Drop‑in hardening for StatisticsPage
Java patch (replace method + add two tiny helpers) // In zms.ataf.ui.pages.statistics.StatisticsPage
import org.openqa.selenium.By;
import org.openqa.selenium.JavascriptExecutor;
import org.openqa.selenium.TimeoutException;
import org.openqa.selenium.WebElement;
import org.openqa.selenium.support.ui.ExpectedConditions;
import org.openqa.selenium.support.ui.WebDriverWait;
import java.time.Duration;
// Helper: wait for any of the given locators to become visible/clickable
private WebElement waitFirstClickable(By[] locators, int seconds) {
WebDriverWait wait = new WebDriverWait(DRIVER, Duration.ofSeconds(seconds));
for (By by : locators) {
try {
return wait.until(ExpectedConditions.elementToBeClickable(by));
} catch (Exception ignored) {}
}
return null;
}
private boolean waitAnyVisible(By[] locators, int seconds) {
WebDriverWait wait = new WebDriverWait(DRIVER, Duration.ofSeconds(seconds));
for (By by : locators) {
try {
wait.until(ExpectedConditions.presenceOfElementLocated(by));
return true;
} catch (Exception ignored) {}
}
return false;
}
public void clickOnApplySelectionButton() {
ScenarioLogManager.getLogger().info("Trying to click on \"Auswahl bestätigen\" button...");
// Make sure the context window is correct and spinners are gone
CONTEXT.set();
if (CONTEXT instanceof StatisticsPageContext s) {
s.waitForSpinners();
}
// Support both <button> and <input type='submit'> variants and labels
By[] buttonCandidates = new By[] {
By.xpath("//button[`@type`='submit' and (normalize-space()='Weiter' or normalize-space()='Auswahl bestätigen')]"),
By.xpath("//input[`@type`='submit' and (translate(`@value`,'WEITER','weiter')='weiter' or contains(translate(`@value`,'ÄAUSW','äausw'),'auswahl'))]"),
By.cssSelector("form button[type='submit'], form input[type='submit']")
};
WebElement submit = waitFirstClickable(buttonCandidates, 20);
Assert.assertNotNull(submit, "Could not find the statistics location submit button.");
try {
scrollToCenterByVisibleElement(submit);
submit.click();
} catch (Exception e) {
ScenarioLogManager.getLogger().warn("Normal click failed (" + e.getClass().getSimpleName() + "). Falling back to JS click.");
((JavascriptExecutor) DRIVER).executeScript("arguments[0].click();", submit);
}
// After submit, wait for a stable post‑condition (overview, sidebar, or sub‑page filter)
By[] postConditions = new By[] {
By.xpath("//h1[contains(normalize-space(),'Übersicht')]"),
By.xpath("//aside//a[contains(normalize-space(),'Kundenstatistik')]"),
By.xpath("//aside//a[contains(normalize-space(),'Dienstleistungsstatistik')]"),
By.xpath("//*[self::button or self::input][normalize-space()='Übernehmen' or translate(`@value`,'ÜBERNEHMEN','übernehmen')='übernehmen']")
};
boolean ok = waitAnyVisible(postConditions, 60);
Assert.assertTrue(ok, "Could not navigate to statistics page after clicking \"Auswahl bestätigen\".");
}Why this will unstick Dienstleistungsstatistik
If you want, I can also push the same “post‑click wait” pattern into clickOnLoginButton to turn the Keycloak 30s timeout into a warning consistently (we already do this partially). On the earlier strategic question (expiring opening-hours test data)
Recommendation: freeze “today” in zmsautomation-test (fastest path, single knob), and also change the Flyway V10 opening-hours inserts to CURDATE()…DATE_ADD(CURDATE(), INTERVAL N DAY) to remove any residual drift. That combination has been robust in CI for similar suites. Would you like me to stage the StatisticsPage.java change above and (optionally) a tiny Flyway tweak that replaces hardcoded opening-hour dates with CURDATE()/DATE_ADD()? If yes, I’ll prepare a minimal plan next. |
Member
Author
|
If you want, I can also push the same “post‑click wait” pattern into clickOnLoginButton to turn the Keycloak 30s timeout into a warning consistently (we already do this partially). The setup should be like in zmsadmin @coderabbitai |
Contributor
|
Tip For best results, initiate chat on the files or code changes.
Java patch (StatisticsPage.java)
// imports to add if missing
import org.openqa.selenium.By;
import org.openqa.selenium.JavascriptExecutor;
import org.openqa.selenium.TimeoutException;
import org.openqa.selenium.WebElement;
import org.openqa.selenium.support.ui.ExpectedConditions;
import org.openqa.selenium.support.ui.WebDriverWait;
import java.time.Duration;
// Replace/strengthen this method
public void clickOnLoginButton() throws Exception {
ScenarioLogManager.getLogger().info("Trying to click on \"Login\" button (Keycloak)...");
CONTEXT.set();
// If we’re not yet on the SSO page, click the entry “Anmelden” on the Statistik landing
if (!isWebElementVisible(5, "//*[`@id`='username' or `@id`='kc-login']", LocatorType.XPATH, false)) {
// robust entry login (button or link)
By[] entryCandidates = new By[] {
By.xpath("//a[contains(normalize-space(),'Anmelden') or contains(.,'Login')]"),
By.cssSelector("a.button--login, a[href*='keycloak'], a[href*='oidc']"),
By.xpath("//button[contains(normalize-space(),'Anmelden') or contains(normalize-space(),'Login')]")
};
WebElement entry = waitFirstClickable(entryCandidates, 15);
Assert.assertNotNull(entry, "Could not find the statistics SSO entry button/link.");
scrollToCenterByVisibleElement(entry);
try {
entry.click();
} catch (Exception e) {
((JavascriptExecutor) DRIVER).executeScript("arguments[0].click();", entry);
}
}
// Now on Keycloak form: fill credentials
enterTextInWebElement(DEFAULT_EXPLICIT_WAIT_TIME, "username", LocatorType.ID,
TestPropertiesHelper.getString("userName"));
enterTextInWebElement(DEFAULT_EXPLICIT_WAIT_TIME, "password", LocatorType.ID,
TestPropertiesHelper.getString("userPassword"));
WebElement submit = waitFirstClickable(new By[] { By.id("kc-login") }, 20);
Assert.assertNotNull(submit, "Could not find Keycloak submit button (kc-login).");
// Click + tolerate slow navigation: warn on timeout if post-login UI is present
try {
scrollToCenterByVisibleElement(submit);
submit.click();
waitAfterSsoSubmit(); // normal fast path
} catch (TimeoutException te) {
ScenarioLogManager.getLogger().warn("SSO login navigation took longer than expected; probing post-login state...");
if (!waitForStatisticsHomeOrFilter(30)) {
// still nowhere → propagate the failure
throw te;
}
} catch (Exception e) {
ScenarioLogManager.getLogger().warn("Normal click failed (" + e.getClass().getSimpleName() + "). Falling back to JS click.");
((JavascriptExecutor) DRIVER).executeScript("arguments[0].click();", submit);
if (!waitForStatisticsHomeOrFilter(60)) {
Assert.fail("Could not navigate to statistics page after SSO JS click.");
}
}
}
// Waits up to 60s for any “good” post-login signal in Statistik
private void waitAfterSsoSubmit() {
Assert.assertTrue(waitForStatisticsHomeOrFilter(60),
"Post-login statistics page did not appear in time.");
}
// Any-of post conditions: Standort select, submit button, or sidebar links
private boolean waitForStatisticsHomeOrFilter(int seconds) {
WebDriverWait wait = new WebDriverWait(DRIVER, Duration.ofSeconds(seconds));
// Let overlays finish if present
try { wait.until(ExpectedConditions.invisibilityOfElementLocated(By.cssSelector(".spinner, div.spinner, .loading"))); }
catch (Exception ignored) { }
By[] goodSignals = new By[] {
By.name("scope"), By.id("scope"),
By.xpath("//button[normalize-space()='Auswahl bestätigen' or normalize-space()='Weiter']"),
By.cssSelector("form button[type='submit'], form input[type='submit']"),
By.xpath("//aside//a[contains(normalize-space(),'Kundenstatistik') or contains(normalize-space(),'Dienstleistungsstatistik')]")
};
for (By by : goodSignals) {
try {
wait.until(ExpectedConditions.presenceOfElementLocated(by));
return true;
} catch (Exception ignored) { }
}
return false;
}
// Small utility to mirror zmsadmin pattern
private WebElement waitFirstClickable(By[] locators, int seconds) {
WebDriverWait w = new WebDriverWait(DRIVER, Duration.ofSeconds(seconds));
for (By by : locators) {
try { return w.until(ExpectedConditions.elementToBeClickable(by)); }
catch (Exception ignored) { }
}
return null;
}Notes
Confirm and I’ll proceed to stage these updates (and, if you like, I can also port the same post‑click wait into clickOnApplySelectionButton for Dienstleistungsstatistik, but from your latest runs only the login path was still occasionally timing out).
✏️ Learnings added
💡 Use |
…st-click wait and JS fallback - Add waitForSpinners() to StatisticsPageContext (align with zmsadmin) - clickOnLoginButton: waitFirstClickable for kc-login, JS fallback, post-login wait for scope/sidebar; treat Keycloak navigation timeout as non-fatal when post-condition appears - clickOnApplySelectionButton: broad locators (Weiter/Auswahl bestätigen), wait for spinners, JS click fallback, wait up to 60s for overview/sidebar/Übernehmen
Member
Author
|
@coderabbitai I don't understand why |
Contributor
|
Tip For best results, initiate chat on the files or code changes.
Why
Two quick hardenings that remove the remaining flakes
Java patch (StatisticsPage.java) // imports (if missing)
import org.openqa.selenium.By;
import org.openqa.selenium.JavascriptExecutor;
import org.openqa.selenium.StaleElementReferenceException;
import org.openqa.selenium.TimeoutException;
import org.openqa.selenium.WebElement;
import org.openqa.selenium.support.ui.ExpectedConditions;
import org.openqa.selenium.support.ui.WebDriverWait;
import java.time.Duration;
// Replaces existing clickOnApplySelectionButton()
public void clickOnApplySelectionButton() {
ScenarioLogManager.getLogger().info("Trying to click on \"Auswahl bestätigen\" button...");
CONTEXT.set();
if (CONTEXT instanceof StatisticsPageContext s) {
s.waitForSpinners();
}
By[] submitCandidates = new By[] {
By.xpath("//button[`@type`='submit' and (normalize-space()='Weiter' or normalize-space()='Auswahl bestätigen')]"),
By.xpath("//input[`@type`='submit' and (translate(`@value`,'WEITER','weiter')='weiter' "
+ "or contains(translate(`@value`,'ÄAUSW','äausw'),'auswahl'))]"),
By.cssSelector("form button[type='submit'], form input[type='submit']")
};
WebDriverWait wait = new WebDriverWait(DRIVER, Duration.ofSeconds(DEFAULT_EXPLICIT_WAIT_TIME));
int attempts = 0;
while (attempts++ < 3) {
try {
WebElement submit = waitFirstClickable(submitCandidates, DEFAULT_EXPLICIT_WAIT_TIME);
Assert.assertNotNull(submit, "Could not find the statistics location submit button.");
scrollToCenterByVisibleElement(submit);
// pre-click: ensure overlays gone
try { wait.until(ExpectedConditions.invisibilityOfElementLocated(By.cssSelector(".spinner, div.spinner, .loading"))); }
catch (Exception ignored) {}
try {
submit.click();
} catch (Exception e) {
ScenarioLogManager.getLogger().warn("Normal click failed (" + e.getClass().getSimpleName() + "). Falling back to JS click.");
((JavascriptExecutor) DRIVER).executeScript("arguments[0].click();", submit);
}
// post-click: either element becomes stale OR we see a next-state signal
boolean navigated = false;
try {
new WebDriverWait(DRIVER, Duration.ofSeconds(10))
.until(ExpectedConditions.stalenessOf(submit));
navigated = true;
} catch (Exception ignored) {
navigated = waitForNextStatisticsState(60);
}
if (navigated) return;
} catch (StaleElementReferenceException sere) {
ScenarioLogManager.getLogger().warn("Submit element went stale; retrying (" + attempts + "/3)...");
} catch (TimeoutException te) {
ScenarioLogManager.getLogger().warn("Timeout waiting around submit; retrying (" + attempts + "/3)...");
}
}
Assert.fail("Could not proceed after clicking \"Auswahl bestätigen\" (retries exhausted).");
}
private WebElement waitFirstClickable(By[] locators, int seconds) {
WebDriverWait w = new WebDriverWait(DRIVER, Duration.ofSeconds(seconds));
for (By by : locators) {
try { return w.until(ExpectedConditions.elementToBeClickable(by)); }
catch (Exception ignored) {}
}
return null;
}
private boolean waitForNextStatisticsState(int seconds) {
WebDriverWait w = new WebDriverWait(DRIVER, Duration.ofSeconds(seconds));
By[] good = new By[] {
// overview or sidebar ready
By.xpath("//h1[contains(normalize-space(),'Übersicht')]"),
By.xpath("//aside//a[contains(normalize-space(),'Kundenstatistik') or contains(normalize-space(),'Dienstleistungsstatistik')]"),
// sub-page filter present (Übernehmen visible)
By.xpath("//*[self::button or self::input][normalize-space()='Übernehmen' "
+ "or translate(`@value`,'ÜBERNEHMEN','übernehmen')='übernehmen']")
};
for (By by : good) {
try { w.until(ExpectedConditions.presenceOfElementLocated(by)); return true; }
catch (Exception ignored) {}
}
return false;
}
Java patch (StatisticsSteps.java) // Add helper
private String computePreviousBusinessDayLocally() {
ZoneId tz = ZoneId.of("Europe/Berlin");
LocalDate d = LocalDate.now(tz).minusDays(1);
if (d.getDayOfWeek() == DayOfWeek.SUNDAY) d = d.minusDays(2);
if (d.getDayOfWeek() == DayOfWeek.SATURDAY) d = d.minusDays(1);
return d.format(DateTimeFormatter.ofPattern("dd.MM.yyyy"));
}
// In zeige_kunden_statistik_fuer_vorherigen_tag(...)
String useOffline = System.getProperty("zms.stat.offlineHolidays",
System.getenv().getOrDefault("ZMS_STAT_OFFLINE_HOLIDAYS", "true"));
String date;
if ("true".equalsIgnoreCase(useOffline)) {
date = computePreviousBusinessDayLocally();
} else {
try {
date = HolidayUtil.getPreviousBusinessDay("by") // existing
.format(DateTimeFormatter.ofPattern("dd.MM.yyyy"));
} catch (Exception ex) {
ScenarioLogManager.getLogger().warn("Holiday API failed; falling back to local previous business day. Cause: " + ex.getMessage());
date = computePreviousBusinessDayLocally();
}
}
// then use 'date' to drive the assertionsLow‑effort environment tweaks that also help
Would you like me to stage the two code changes above? They’re small and should eliminate the remaining flakes:
💡 Use |
Member
Author
@coderabbitai total nonsense. It logs in and selects worstation just like zmsadmin (This is where we have timeout issues unlike zmsadmin). The part where it selects a location the second time from the selection scroll window and date range is after navigating to the Kundenstatistik or Dienstleistungstatistik. Looks like you're not up to date. |
Contributor
|
Tip For best results, initiate chat on the files or code changes.
Here’s a focused fix to make zmsstatistic behave like zmsadmin at that exact point.
Java (replace method body and add the two helpers) // imports to have:
// import org.openqa.selenium.*;
// import org.openqa.selenium.support.ui.*;
// import java.time.Duration;
public void clickOnLoginButton() throws Exception {
ScenarioLogManager.getLogger().info("Trying to click on \"Login\" button (Keycloak)...");
CONTEXT.set();
// If we are on Keycloak already, fields are present; otherwise click the entry link/button first
if (!isWebElementVisible(5, "//*[`@id`='username' or `@id`='kc-login']", LocatorType.XPATH, false)) {
By[] entryCandidates = new By[] {
By.xpath("//a[contains(normalize-space(),'Anmelden') or contains(.,'Login')]"),
By.cssSelector("a.button--login, a[href*='keycloak'], a[href*='oidc']"),
By.xpath("//button[contains(normalize-space(),'Anmelden') or contains(normalize-space(),'Login')]")
};
WebElement entry = waitFirstClickable(entryCandidates, DEFAULT_EXPLICIT_WAIT_TIME);
Assert.assertNotNull(entry, "Could not find the statistics SSO entry button/link.");
scrollToCenterByVisibleElement(entry);
try { entry.click(); } catch (Exception e) { ((JavascriptExecutor) DRIVER).executeScript("arguments[0].click();", entry); }
}
// Fill credentials and click kc-login
enterTextInWebElement(DEFAULT_EXPLICIT_WAIT_TIME, "username", LocatorType.ID, TestPropertiesHelper.getString("userName"));
enterTextInWebElement(DEFAULT_EXPLICIT_WAIT_TIME, "password", LocatorType.ID, TestPropertiesHelper.getString("userPassword"));
WebElement submit = waitFirstClickable(new By[]{ By.id("kc-login") }, DEFAULT_EXPLICIT_WAIT_TIME);
Assert.assertNotNull(submit, "Could not find Keycloak submit button (kc-login).");
try {
scrollToCenterByVisibleElement(submit);
submit.click();
Assert.assertTrue(waitForStatisticsHomeOrFilter(60), "Post-login statistics page did not appear in time.");
} catch (TimeoutException te) {
ScenarioLogManager.getLogger().warn("SSO login navigation took longer than expected; probing post-login state...");
Assert.assertTrue(waitForStatisticsHomeOrFilter(30), "Post-login UI not detected after timeout.");
} catch (Exception e) {
ScenarioLogManager.getLogger().warn("Normal kc-login click failed (" + e.getClass().getSimpleName() + "). Falling back to JS click.");
((JavascriptExecutor) DRIVER).executeScript("arguments[0].click();", submit);
Assert.assertTrue(waitForStatisticsHomeOrFilter(60), "Post-login UI not detected after JS click.");
}
}
private WebElement waitFirstClickable(By[] locators, int seconds) {
WebDriverWait w = new WebDriverWait(DRIVER, Duration.ofSeconds(seconds));
for (By by : locators) {
try { return w.until(ExpectedConditions.elementToBeClickable(by)); } catch (Exception ignored) {}
}
return null;
}
private boolean waitForStatisticsHomeOrFilter(int seconds) {
WebDriverWait w = new WebDriverWait(DRIVER, Duration.ofSeconds(seconds));
try { w.until(ExpectedConditions.invisibilityOfElementLocated(By.cssSelector(".spinner, div.spinner, .loading"))); } catch (Exception ignored) {}
By[] goodSignals = new By[] {
By.name("scope"), By.id("scope"), // Standort select present
By.xpath("//button[normalize-space()='Auswahl bestätigen' or normalize-space()='Weiter']"),
By.cssSelector("form button[type='submit'], form input[type='submit']"),
By.xpath("//aside//a[contains(normalize-space(),'Kundenstatistik') or contains(normalize-space(),'Dienstleistungsstatistik')]")
};
for (By by : goodSignals) {
try { w.until(ExpectedConditions.presenceOfElementLocated(by)); return true; } catch (Exception ignored) {}
}
return false;
}
Java (replace clickOnApplySelectionButton) public void clickOnApplySelectionButton() {
ScenarioLogManager.getLogger().info("Trying to click on \"Auswahl bestätigen\" button...");
CONTEXT.set();
if (CONTEXT instanceof StatisticsPageContext s) { s.waitForSpinners(); }
By[] submitCandidates = new By[] {
By.xpath("//button[`@type`='submit' and (normalize-space()='Weiter' or normalize-space()='Auswahl bestätigen')]"),
By.xpath("//input[`@type`='submit' and (translate(`@value`,'WEITER','weiter')='weiter' or contains(translate(`@value`,'ÄAUSW','äausw'),'auswahl'))]"),
By.cssSelector("form button[type='submit'], form input[type='submit']")
};
WebDriverWait wait = new WebDriverWait(DRIVER, Duration.ofSeconds(DEFAULT_EXPLICIT_WAIT_TIME));
for (int attempt = 1; attempt <= 3; attempt++) {
try {
WebElement submit = waitFirstClickable(submitCandidates, DEFAULT_EXPLICIT_WAIT_TIME);
Assert.assertNotNull(submit, "Could not find the statistics location submit button.");
scrollToCenterByVisibleElement(submit);
try { wait.until(ExpectedConditions.invisibilityOfElementLocated(By.cssSelector(".spinner, div.spinner, .loading"))); } catch (Exception ignored) {}
try { submit.click(); }
catch (Exception e) {
ScenarioLogManager.getLogger().warn("Normal click failed (" + e.getClass().getSimpleName() + "). Falling back to JS click.");
((JavascriptExecutor) DRIVER).executeScript("arguments[0].click();", submit);
}
boolean navigated = false;
try { new WebDriverWait(DRIVER, Duration.ofSeconds(10)).until(ExpectedConditions.stalenessOf(submit)); navigated = true; }
catch (Exception ignored) { navigated = waitForNextStatisticsState(60); }
if (navigated) return;
ScenarioLogManager.getLogger().warn("Post-submit state not detected; retrying (" + attempt + "/3)...");
} catch (StaleElementReferenceException | TimeoutException e) {
ScenarioLogManager.getLogger().warn("Submit interaction issue (" + e.getClass().getSimpleName() + "); retrying (" + attempt + "/3)...");
}
}
Assert.fail("Could not proceed after clicking \"Auswahl bestätigen\" (retries exhausted).");
}
private boolean waitForNextStatisticsState(int seconds) {
WebDriverWait w = new WebDriverWait(DRIVER, Duration.ofSeconds(seconds));
By[] next = new By[] {
By.xpath("//h1[contains(normalize-space(),'Übersicht')]"),
By.xpath("//aside//a[contains(normalize-space(),'Kundenstatistik') or contains(normalize-space(),'Dienstleistungsstatistik')]"),
By.xpath("//*[self::button or self::input][normalize-space()='Übernehmen' or translate(`@value`,'ÜBERNEHMEN','übernehmen')='übernehmen']")
};
for (By by : next) {
try { w.until(ExpectedConditions.presenceOfElementLocated(by)); return true; } catch (Exception ignored) {}
}
return false;
}This directly addresses the two flaky points you called out:
If you want, I can also lift the exact SSO/login helper out of AdminPage to a shared utility so Admin/Statistic stay in lock‑step. Quick answer to your earlier question (data freshness)
Want me to stage the two StatisticsPage changes above? I’ll prep a minimal plan and wire it exactly like in zmsadmin. Task: Stabilize zmsstatistic first-screen flows (SSO + initial Standort submit)
✏️ Learnings added
💡 Use |
Member
Author
|
@coderabbitai I still don't think you understand we have two different scope selections. The first one is like zmsadmin to select the workstation. The second is to apply to the statistics table and may be rendered differently from the first selection box. |
Contributor
|
Tip For best results, initiate chat on the files or code changes.
Here’s a concise refactor plan to make that separation explicit and robust.
Example core of the two helpers (shortened to show intent) // Detect landing form vs subpage filter
private boolean isInitialScopeFormPresent() {
return isWebElementVisible(3, "//form//select[`@name`='scope' or `@id`='scope']",
LocatorType.XPATH, false)
|| isWebElementVisible(3, "//label[contains(.,'Standort')]", LocatorType.XPATH, false);
}
private boolean isStatisticsFilterPresent() {
return isWebElementVisible(3, "//*[self::button or self::input][normalize-space()='Übernehmen' or translate(`@value`,'ÜBERNEHMEN','übernehmen')='übernehmen']",
LocatorType.XPATH, false)
|| isWebElementVisible(3, "#scope-select, .select2, .ts-control", LocatorType.CSS, false);
}
public void selectInitialScopeAndApply(String location, String workstationOrNull) {
CONTEXT.set();
if (CONTEXT instanceof StatisticsPageContext s) s.waitForSpinners();
// Standort
By[] scopeSel = {
By.cssSelector("select[name='scope']"),
By.id("scope"),
By.xpath("//label[contains(normalize-space(.),'Standort')]/following::select[1]")
};
WebElement scope = waitFirstClickable(scopeSel, DEFAULT_EXPLICIT_WAIT_TIME);
Assert.assertNotNull(scope, "Initial Standort select not found.");
selectDropDownListValueByVisibleText(DEFAULT_EXPLICIT_WAIT_TIME, scope, location);
// Optional Tresen/Platz-Nr.
By[] wsInp = {
By.cssSelector("input[name='workstation']"),
By.id("tresen"),
By.xpath("//label[matches(normalize-space(.),'(?i)(Platz|Tresen)')]/following::input[1]")
};
WebElement ws = waitFirstClickable(wsInp, 3);
if (ws != null && workstationOrNull != null && !workstationOrNull.isBlank()) {
enterTextInWebElement(DEFAULT_EXPLICIT_WAIT_TIME, ws, workstationOrNull);
}
// Submit with anti-stale
clickSubmitWithRetries(new By[]{
By.xpath("//button[`@type`='submit' and (normalize-space()='Weiter' or normalize-space()='Auswahl bestätigen')]"),
By.cssSelector("form button[type='submit'], form input[type='submit']")
});
}
public void applyStatisticsTableFilter(String location, LocalDate from, LocalDate to) {
CONTEXT.set();
if (!isStatisticsFilterPresent()) return;
// Scope on subpage: native select[] or plugin container
if (isWebElementVisible(2, "select[name='scope[]'], select[name='scope']", LocatorType.CSS, false)) {
selectDropDownListValueByVisibleText(DEFAULT_EXPLICIT_WAIT_TIME,
"select[name='scope[]'], select[name='scope']", LocatorType.CSS, location);
} else {
// plugin: select2 / tom-select
clickOnWebElement(DEFAULT_EXPLICIT_WAIT_TIME, "#scope-select, .select2, .ts-control", LocatorType.CSS, false);
enterTextInWebElement(DEFAULT_EXPLICIT_WAIT_TIME,
".ts-control input, .select2-search__field", LocatorType.CSS, location);
clickOnWebElement(DEFAULT_EXPLICIT_WAIT_TIME,
String.format("//div[contains(`@class`,'option') or contains(`@class`,'select2-results__option')][normalize-space()='%s']", location),
LocatorType.XPATH, false);
}
// Dates via JS (names/ids may differ)
setDateByJs(new String[]{"von","from"}, from);
setDateByJs(new String[]{"bis","to"}, to);
// Übernehmen with anti-stale and post-condition wait
clickSubmitWithRetries(new By[]{
By.xpath("//*[self::button or self::input][normalize-space()='Übernehmen' or translate(`@value`,'ÜBERNEHMEN','übernehmen')='übernehmen']")
});
}This keeps the flows independent and variant-aware (native select vs plugin; date ids vs names), and uses the same anti-stale + spinner + JS‑fallback strategy as zmsadmin. If you confirm, I’ll wire this in exactly as above and adjust the steps to call:
Also, I’ll keep the existing “any-of” post-condition waits to eliminate the remaining timeouts. Please confirm:
✏️ Learnings added
💡 Use |
…selections - Login and first scope (selectLocation, clickOnApplySelectionButton) match zmsadmin; JS click on confirm to avoid navigation timeout and stale element - Second scope: rename applyLocationAndDateFilter to selectScopeInStatisticsTableFilter (statistics table filter only, after opening Kundenstatistik/Dienstleistungsstatistik) - Expose STATISTICS_TIMEOUT_SECONDS for overview wait after apply selection
Member
Author
|
@coderabbitai check last commit to see speration of concerns now |
Contributor
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: Length of output: 22194
What changed and looks correct
|
Member
Author
|
Still randomly fails at work station select @coderabbitai 
|
Contributor
|
Tip For best results, initiate chat on the files or code changes. The Comment image shows the zmsstatistic initial selection screen (first scope), with the Standort dropdown and the workstation (“Platz-Nr. oder Tresen”) input above an “Auswahl bestätigen”/“Weiter” submit. This is the same form as zmsadmin’s landing workflow before navigating to any statistics pages.
Below is a concise, drop-in hardening for StatisticsPage that mirrors zmsadmin’s robustness.
private WebElement waitFirstClickable(By[] locators, int seconds) { private boolean waitForStatisticsHomeOrFilter(int seconds) { }
private boolean waitForNextStatisticsState(int seconds) { Why this addresses the random failures
Optional quick win (no code change)
If you want, I can prep a tiny follow-up to add a waitForSidebarReady() before clicking “Kundenstatistik/Dienstleistungsstatistik,” but based on your last run the second-screen flow is already stable. Would you like me to stage these two StatisticsPage changes? If yes, I’ll prepare a small plan to land them.
✏️ Learnings added
💡 Use |
Contributor
|
Tip For best results, initiate chat on the files or code changes. The Comment image shows the zmsstatistic landing form where you select the Standort and (optionally) enter the “Platz‑Nr. oder Tresen,” with a primary submit button labeled “Auswahl bestätigen” or “Weiter.” This is the first scope/workstation screen that mirrors zmsadmin before you navigate to Kunden‑/Dienstleistungsstatistik. Based on your latest logs:
Two targeted hardenings that remove these flakes
// In zmsautomation/src/test/java/zms/ataf/ui/pages/statistics/StatisticsPage.java
// Add if missing:
import org.openqa.selenium.By;
import org.openqa.selenium.JavascriptExecutor;
import org.openqa.selenium.TimeoutException;
import org.openqa.selenium.WebElement;
import org.openqa.selenium.support.ui.ExpectedConditions;
import org.openqa.selenium.support.ui.WebDriverWait;
import java.time.Duration;
private boolean waitForInitialScopeOrSidebar(int seconds) {
WebDriverWait w = new WebDriverWait(DRIVER, Duration.ofSeconds(seconds));
// Let overlays settle if present
try { w.until(ExpectedConditions.invisibilityOfElementLocated(By.cssSelector(".spinner, div.spinner, .loading"))); } catch (Exception ignored) {}
By[] good = new By[] {
// initial scope form (first screen)
By.name("scope"), By.id("scope"),
By.xpath("//button[normalize-space()='Auswahl bestätigen' or normalize-space()='Weiter']"),
// or the sidebar already rendered
By.xpath("//aside//a[contains(normalize-space(),'Kundenstatistik') or contains(normalize-space(),'Dienstleistungsstatistik')]")
};
for (By by : good) {
try { w.until(ExpectedConditions.presenceOfElementLocated(by)); return true; } catch (Exception ignored) {}
}
return false;
}
public void clickOnLoginButton() throws Exception {
ScenarioLogManager.getLogger().info("Trying to click on \"Login\" button (Keycloak)...");
CONTEXT.set();
// We are already on the Keycloak page at this point in your flow:
enterTextInWebElement(DEFAULT_EXPLICIT_WAIT_TIME, "username", LocatorType.ID, TestPropertiesHelper.getString("userName"));
enterTextInWebElement(DEFAULT_EXPLICIT_WAIT_TIME, "password", LocatorType.ID, TestPropertiesHelper.getString("userPassword"));
WebElement submit = findElementByLocatorType(DEFAULT_EXPLICIT_WAIT_TIME, "kc-login", LocatorType.ID);
scrollToCenterByVisibleElement(submit);
try {
submit.click();
Assert.assertTrue(waitForInitialScopeOrSidebar(60), "Post-login statistics page did not appear in time.");
ScenarioLogManager.getLogger().info("SSO login submitted successfully.");
} catch (TimeoutException te) {
ScenarioLogManager.getLogger().warn("SSO navigation took longer than expected; probing post-login state...");
Assert.assertTrue(waitForInitialScopeOrSidebar(30), "Post-login UI not detected after timeout.");
} catch (Exception e) {
ScenarioLogManager.getLogger().warn("Normal kc-login click failed (" + e.getClass().getSimpleName() + "). Falling back to JS click.");
((JavascriptExecutor) DRIVER).executeScript("arguments[0].click();", submit);
Assert.assertTrue(waitForInitialScopeOrSidebar(60), "Post-login UI not detected after JS click.");
}
}
// Still in StatisticsPage.java
import org.openqa.selenium.StaleElementReferenceException;
private boolean waitForOverviewOrFilter(int seconds) {
WebDriverWait w = new WebDriverWait(DRIVER, Duration.ofSeconds(seconds));
By[] next = new By[] {
By.xpath("//h1[contains(normalize-space(),'Übersicht')]"),
By.xpath("//aside//a[contains(normalize-space(),'Kundenstatistik') or contains(normalize-space(),'Dienstleistungsstatistik')]"),
By.xpath("//*[self::button or self::input][normalize-space()='Übernehmen' or translate(`@value`,'ÜBERNEHMEN','übernehmen')='übernehmen']")
};
for (By by : next) {
try { w.until(ExpectedConditions.presenceOfElementLocated(by)); return true; } catch (Exception ignored) {}
}
return false;
}
public void clickOnApplySelectionButton() {
ScenarioLogManager.getLogger().info("Trying to click on \"Auswahl bestätigen\" button...");
CONTEXT.set();
if (CONTEXT instanceof StatisticsPageContext s) s.waitForSpinners();
By[] candidates = new By[] {
By.xpath("//button[`@type`='submit' and (normalize-space()='Weiter' or normalize-space()='Auswahl bestätigen')]"),
By.cssSelector("form button[type='submit']"),
By.cssSelector("form input[type='submit']")
};
WebDriverWait wait = new WebDriverWait(DRIVER, Duration.ofSeconds(DEFAULT_EXPLICIT_WAIT_TIME));
for (int attempt = 1; attempt <= 3; attempt++) {
try {
WebElement submit = null;
for (By by : candidates) {
try { submit = wait.until(ExpectedConditions.elementToBeClickable(by)); break; } catch (Exception ignored) {}
}
Assert.assertNotNull(submit, "Initial submit button not found.");
// Guard: enabled and no overlay
if (!submit.isEnabled()) {
ScenarioLogManager.getLogger().warn("Submit disabled; waiting for enable...");
wait.until(ExpectedConditions.elementToBeClickable(submit));
}
try { wait.until(ExpectedConditions.invisibilityOfElementLocated(By.cssSelector(".spinner, div.spinner, .loading"))); } catch (Exception ignored) {}
scrollToCenterByVisibleElement(submit);
try {
submit.click();
} catch (Exception e) {
((JavascriptExecutor) DRIVER).executeScript("arguments[0].click();", submit);
}
// Post-submit: element stale or next-state visible
boolean progressed = false;
try { new WebDriverWait(DRIVER, Duration.ofSeconds(10)).until(ExpectedConditions.stalenessOf(submit)); progressed = true; }
catch (Exception ignored) { progressed = waitForOverviewOrFilter(60); }
if (progressed) return;
ScenarioLogManager.getLogger().warn("No post-submit state yet; retrying (" + attempt + "/3)...");
} catch (StaleElementReferenceException | TimeoutException e) {
ScenarioLogManager.getLogger().warn("Submit interaction issue (" + e.getClass().getSimpleName() + "); retrying (" + attempt + "/3)...");
}
}
Assert.fail("Could not proceed after clicking \"Auswahl bestätigen\" (retries exhausted).");
}Optional but recommended
If you want, I can wire these two drop-in changes now so zmsstatistic behaves like zmsadmin at kc-login and at the first submit. Shall I proceed and stage them? |
Avoid navigation timeout after SSO by clicking kc-login via JavaScript and waiting for the workstation select (name=scope) instead of full page load.
Member
Author
|
Okay the first test fails at the second scope selection now. Flacky The second test fails at the zms login page. which doesn't seem correctly loaded because logo is missing, the drop down for password user missing, commit version tag is also missing which would be loaded if the page was rendered correctly. 
@coderabbitai
|
Contributor
|
Tip For best results, initiate chat on the files or code changes. The first image (Comment image 1) shows the statistics subpage filter (second scope) with a scope picker and a date range (Von/Bis) plus an Übernehmen button; the click path toward Kundenstatistik/Dienstleistungsstatistik leads here and is flaky during selection/submit.
What I’ll change
Drop-in patches
// imports to add if missing
import org.openqa.selenium.*;
import org.openqa.selenium.support.ui.*;
import java.time.Duration;
private void waitForSidebarReady() {
WebDriverWait w = new WebDriverWait(DRIVER, Duration.ofSeconds(DEFAULT_EXPLICIT_WAIT_TIME));
By[] probes = {
By.xpath("//aside//a"),
By.cssSelector("nav a, .sidebar a, aside a")
};
for (By by : probes) {
try {
w.until(ExpectedConditions.elementToBeClickable(by));
return;
} catch (Exception ignored) {}
}
}
private void safeClickSidebarLink(String label) {
waitForSidebarReady();
WebDriverWait w = new WebDriverWait(DRIVER, Duration.ofSeconds(DEFAULT_EXPLICIT_WAIT_TIME));
By link = By.xpath("//aside//a[contains(normalize-space(),'" + label + "')]");
int attempts = 0;
while (attempts++ < 3) {
try {
WebElement el = w.until(ExpectedConditions.elementToBeClickable(link));
scrollToCenterByVisibleElement(el);
try {
el.click();
} catch (Exception e) {
((JavascriptExecutor) DRIVER).executeScript("arguments[0].click();", el);
}
// post-click: wait for subpage filter or page heading
if (waitForStatisticsFilterOrHeading(60)) return;
} catch (StaleElementReferenceException | TimeoutException ignored) {}
}
Assert.fail("Could not open '" + label + "' from the sidebar (retries exhausted).");
}
private boolean waitForStatisticsFilterOrHeading(int seconds) {
WebDriverWait w = new WebDriverWait(DRIVER, Duration.ofSeconds(seconds));
By[] ok = new By[] {
// Übernehmen on subpage filter
By.xpath("//*[self::button or self::input][normalize-space()='Übernehmen' or translate(`@value`,'ÜBERNEHMEN','übernehmen')='übernehmen']"),
// Kundenstatistik / Dienstleistungsstatistik page headings
By.xpath("//h1[contains(normalize-space(),'Kundenstatistik')]"),
By.xpath("//h1[contains(normalize-space(),'Dienstleistungsstatistik')]")
};
for (By by : ok) {
try { w.until(ExpectedConditions.presenceOfElementLocated(by)); return true; }
catch (Exception ignored) {}
}
return false;
}
// Use it in the existing methods:
public void clickOnCustomerStatistics() {
ScenarioLogManager.getLogger().info("Trying to click on \"Kundenstatistik\" button in the sidebar...");
safeClickSidebarLink("Kundenstatistik");
}
public void clickOnServiceStatistics() {
ScenarioLogManager.getLogger().info("Trying to click on \"Dienstleistungsstatistik\" button in the sidebar...");
safeClickSidebarLink("Dienstleistungsstatistik");
}
public void selectScopeInStatisticsTableFilter(String location) {
ScenarioLogManager.getLogger().info("Selecting scope in statistics table filter (second scope): \"" + location + "\"");
CONTEXT.set();
// Prefer JS plugin containers, fallback to native select
boolean selected = false;
// tom-select / custom container
if (isWebElementVisible(2, "#scope-select, .ts-control", LocatorType.CSS, false)) {
clickOnWebElement(DEFAULT_EXPLICIT_WAIT_TIME, "#scope-select, .ts-control", LocatorType.CSS, false);
enterTextInWebElement(DEFAULT_EXPLICIT_WAIT_TIME, ".ts-control input", LocatorType.CSS, location);
clickOnWebElement(DEFAULT_EXPLICIT_WAIT_TIME,
"//div[contains(`@class`,'ts-dropdown') or contains(`@class`,'option')][.//text()[normalize-space()='" + location + "']]",
LocatorType.XPATH, false);
selected = isWebElementVisible(3,
"div.ts-control .item, div.ts-control .ts-wrapper.has-items", LocatorType.CSS, false);
}
// select2 fallback
if (!selected && isWebElementVisible(2, ".select2, .select2-selection", LocatorType.CSS, false)) {
clickOnWebElement(DEFAULT_EXPLICIT_WAIT_TIME, ".select2-selection", LocatorType.CSS, false);
enterTextInWebElement(DEFAULT_EXPLICIT_WAIT_TIME, ".select2-search__field", LocatorType.CSS, location);
clickOnWebElement(DEFAULT_EXPLICIT_WAIT_TIME,
"//li[contains(`@class`,'select2-results__option')][normalize-space()='" + location + "']",
LocatorType.XPATH, false);
selected = isWebElementVisible(3,
".select2-selection__choice[title='" + location + "']", LocatorType.CSS, false);
}
// native select fallback
if (!selected && isWebElementVisible(2, "select[name='scope[]'], select[name='scope']", LocatorType.CSS, false)) {
selectDropDownListValueByVisibleText(DEFAULT_EXPLICIT_WAIT_TIME,
"select[name='scope[]'], select[name='scope']", LocatorType.CSS, location);
selected = true;
}
Assert.assertTrue(selected, "Scope \"" + location + "\" could not be selected in the statistics filter.");
}
public void applyDateRangeFilter(LocalDate from, LocalDate to) {
ScenarioLogManager.getLogger().info("Applying date range in statistics table filter...");
CONTEXT.set();
setDateByJs(new String[]{"von","from"}, from);
setDateByJs(new String[]{"bis","to"}, to);
// submit Übernehmen with retry + JS fallback
By submit = By.xpath("//*[self::button or self::input][normalize-space()='Übernehmen' or translate(`@value`,'ÜBERNEHMEN','übernehmen')='übernehmen']");
WebDriverWait w = new WebDriverWait(DRIVER, Duration.ofSeconds(DEFAULT_EXPLICIT_WAIT_TIME));
int attempts = 0;
while (attempts++ < 3) {
try {
WebElement el = w.until(ExpectedConditions.elementToBeClickable(submit));
scrollToCenterByVisibleElement(el);
try { el.click(); } catch (Exception e) { ((JavascriptExecutor) DRIVER).executeScript("arguments[0].click();", el); }
if (waitForStatisticsTable(60)) return;
} catch (StaleElementReferenceException | TimeoutException ignored) {}
}
Assert.fail("Could not apply statistics date filter (retries exhausted).");
}
private void setDateByJs(String[] namesOrIds, LocalDate date) {
String val = date.toString(); // yyyy-MM-dd
WebDriverWait w = new WebDriverWait(DRIVER, Duration.ofSeconds(DEFAULT_EXPLICIT_WAIT_TIME));
for (String key : namesOrIds) {
By byName = By.name(key);
By byId = By.id(key);
try {
WebElement el = null;
try { el = w.until(ExpectedConditions.presenceOfElementLocated(byName)); }
catch (Exception ignored) { el = w.until(ExpectedConditions.presenceOfElementLocated(byId)); }
((JavascriptExecutor)DRIVER).executeScript(
"arguments[0].value=arguments[1];arguments[0].dispatchEvent(new Event('change',{bubbles:true}));",
el, val);
return;
} catch (Exception ignored) {}
}
}
private boolean waitForStatisticsTable(int seconds) {
WebDriverWait w = new WebDriverWait(DRIVER, Duration.ofSeconds(seconds));
By table = By.xpath("//table[contains(`@class`,'table--base')]");
try {
w.until(ExpectedConditions.presenceOfElementLocated(table));
// optionally ensure at least one row is present
w.until(ExpectedConditions.presenceOfElementLocated(By.xpath("//table[contains(`@class`,'table--base')]//tr")));
return true;
} catch (Exception e) {
return false;
}
}
// imports
import org.openqa.selenium.*;
import org.openqa.selenium.support.ui.*;
public void navigateToPage() {
String statisticsUrl = /* existing resolution */;
try {
navigateToPageByUrl(statisticsUrl, DEFAULT_EXPLICIT_WAIT_TIME * 1000);
} catch (TimeoutException te) {
// Probe for UI even after timeout
if (!postNavProbe(10)) {
// One refresh-and-retry
DRIVER.navigate().refresh();
try {
navigateToPageByUrl(statisticsUrl, DEFAULT_EXPLICIT_WAIT_TIME * 1000);
} catch (TimeoutException ignored) {
// Last probe
Assert.assertTrue(postNavProbe(10), "Could not navigate to statistics page!");
}
}
}
Window wnd = getWindows().getOrDefault(TITLE, null);
if (wnd == null) {
wnd = switchToOpenedWindowByTitle(TITLE, DEFAULT_EXPLICIT_WAIT_TIME * 1000);
}
setWindow(wnd);
switchToDefaultContent();
}
private boolean postNavProbe(int seconds) {
WebDriverWait w = new WebDriverWait(DRIVER, Duration.ofSeconds(seconds));
By[] ok = new By[] {
By.id("kc-login"), // Keycloak button present
By.name("scope"), By.id("scope"), // first scope form
By.xpath("//h1[contains(normalize-space(),'Anmeldung - ZMS Statistik')]")
};
for (By by : ok) {
try { w.until(ExpectedConditions.presenceOfElementLocated(by)); return true; }
catch (Exception ignored) {}
}
return false;
}Two small environment tweaks (no code)
Would you like me to stage these changes? I’ll keep the first-scope flow as-is, add the safe sidebar click + robust 2nd-scope filter, and make the statistics navigation tolerant to render timeouts.
• File: zmsautomation/src/test/java/zms/ataf/ui/pages/statistics/StatisticsPage.java
• File: zmsautomation/src/test/java/zms/ataf/ui/pages/statistics/StatisticsPageContext.java
• Script (optional): zmsautomation/zmsautomation-test
✏️ Learnings added
💡 Use |
Contributor
|
Tip For best results, initiate chat on the files or code changes. The first image shows the statistics subpage filter (second scope) after you click “Kundenstatistik”/“Dienstleistungsstatistik”: a scope picker (often plugin-based), Von/Bis date inputs, and an “Übernehmen” button. This is where the second-scope selection is flaky. The second and third images show the ZMS Statistik landing/login not fully rendered: header logo missing, no user/password dropdown, and the usual commit/version tag is absent. That aligns with the navigation timeouts you see (page not fully loaded). Based on the logs and screenshots, two stabilization points remain:
Proposed hardening (drop-in, focused on the two trouble spots)
|
…lick for sidebar - StatisticsPageContext: set page load timeout to 60s before initial navigation - clickOnCustomerStatistics/clickOnServiceStatistics: JS click + wait for sub-page heading to avoid navigation timeout when opening Kundenstatistik/Dienstleistungsstatistik (Defensive; if failures started this morning, env/load may be the root cause.)
…dmin Login page uses main.twig; dropping the external fontawesome.com request avoids slow/blocked CDN blocking document load and flaky UI tests.
Remove STATISTICS_TIMEOUT_SECONDS and pageLoadTimeout; use DEFAULT_EXPLICIT_WAIT_TIME for navigation, window switch, and all post-click waits (login scope, overview, sidebar links).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This commit implements the ATAF (Test Automation Framework) integration for ZMS API automation tests. The new zmsautomation module is a copy of zmsapiautomation with ATAF/Cucumber integration, while the original zmsapiautomation module remains unchanged for CI compatibility.
Phase 1: Project Setup
Phase 2: Configuration Files
Phase 3: ATAF Integration Code
Phase 4: Feature Files
Phase 5: Documentation
Phase 6: Migration Examples
Key Features:
The zmsapiautomation module remains unchanged and continues to run in GitHub Actions CI. The zmsautomation module is ready for local testing with the ATAF profile (requires Artifactory access). Both modules will coexist until Phase 7 when ATAF becomes open source and CI can be updated.
Pull Request Checklist (Feature Branch to
next):nextBranch in meinen Feature-Branch gemergt.Summary by CodeRabbit
API Testing Framework
UI Testing Framework
Database Automation
Test Scenarios
Environment Setup
Documentation