Feedback - Lab 2¶

Tag your final code submission¶

When evaluating your code, we use the Lab2 tag to checkout your code, as in git checkout Lab2. If you did not tag your final code submission, we have to search for the last commit before the deadline.

Some groups however tagged code correctly, but made changes after the tag, before the deadline. This makes it hard for us to evaluate your code, since we don't know which commit to use. Please use other tags to test or mark progress and reserve the requested LabX tags for the final submission.

Tip: it is possible to remove a tag and re-add it to a different commit if you mistagged your code.

Maven lifecycle phases¶

Running both compile and package in one job¶

In maven-build many added the ./mvnw package instruction instead of replacing the ./mvnw compile instruction. This is not necessary, since ./mvnw package will also compile the code. Luckily maven will skip the compile phase if the code is already compiled, but it does cost you a bit of processing overhead since you start the command twice.

maven-build:
  script:
    # - mvn compile ## No longer necessary since package will also compile
    - mvn package -DskipTests

Keep your Unit Tests small¶

Many groups write a Unit Test which tests e.g. all functionality of a Cleric or Miner. Maybe this is because of confusion between the term Unit Test and the DevOps game Units. A Unit Test should test a single unit of code, e.g. a single method. If you want to test all functionality of a Cleric or Miner, you should write multiple Unit Tests, each testing a single method. This makes it easier to find the cause of a failing test and makes it easier to write the test. If you want to avoid code duplication in your Unit Tests, you can use the @Before annotation to create a setup method which is run before each test or extract functionality to a separate method. You can go even further and make additional test suites, one for each Game Unit for instance, to group common functionality together.

Using the wrong image version in the integration test¶

We noticed a fair amount of integration tests still using the latest version of the image instead of the commit hash. This essentially means that you are not testing the code of the commit that triggered the pipeline, but a quasi random version since as the pipeline is set up now, multiple branches can push that latest tag.

Excessive use of YAML anchors and extends¶

The idea of reducing duplication is to A) make the code more readable and B) make it easier to change. If you use anchors and extends too much you will achieve the opposite. Some groups went really hard into this subject but ended up with a very complex .gitlab-ci.yml. Their pipelines worked, and they (often arguably) reduced duplication, but at the cost of readability, maintainability and also collaboration.

Below an example of how you can achieve a clean and readable integration test setup with only extends:

.integration-tmpl:
  cache: []
  dependencies: []
  stage: tests
  image:
    name: ${UTIL_REGISTRY}/devops-runner:latest
    entrypoint:
      - ""
  script:
    - java -cp /app/resources:/app/classes:/app/libs/* be.ugent.devops.gamehost.services.runner.LauncherKt
  services:
    - name: ${CI_REGISTRY_IMAGE}/logic-service:${CI_COMMIT_SHORT_SHA}
      alias: logic-service
  variables:
    PLAYER_NAME: "CI/CD Player"
    LOGIC_URL: http://logic-service:8080
    ENABLE_FETCH_LOGS: "true"
    ABORT_ON_LOGIC_ERROR: "true"

test-integration-default:
  extends: .integration-tmpl
  variables:
    TURN_INTERVAL_MS: 25
    TURN_LIMIT: 100
    CPU_PLAYERS: 5
    MAP_WIDTH: 50
    MAP_HEIGHT: 50

test-integration-tiny-duel:
  extends: .integration-tmpl
  variables:
    TURN_INTERVAL_MS: 25
    TURN_LIMIT: 1000
    CPU_PLAYERS: 1
    MAP_WIDTH: 5
    MAP_HEIGHT: 5

Another tip is to use the GitLab pipeline editor and its Full Configuration view which will render the .gitlab-ci.yml file in its entirety, processing all anchors and extends. This way you can see what the final pipeline will look like and you can check if it does what you expect it to do.

Mixing Quarkus configuration options¶

It is possible to configure Quarkus and your Logic Service using multiple methods. You can use environment variables, system properties, application.properties, application.yaml, profiles, etc. However, it is important to be intentional and consistent in how you configure your service.

In some jobs we see mixing of environment variables and system properties for the same configuration option. Setting both the variable QUARKUS_CONTAINER_IMAGE_PUSH=true and the system property -Dquarkus.container-image.push=true will lead to the system property taking precedence. Now when you decide to change the configuration, you might change the environment variable, but the system property will still be set and take precedence. This can lead to unexpected behavior and confusion.

Clean-up policy: case sensitivity¶

Be mindful that the regex for the clean up policy is case sensitive! Many groups had a policy such as lab.* but then pushedLab 2. This will not match and thus the tag will be removed.

An example regex which sets case insensitivity and matches both none and some whitespace is (?i)lab ?\d+, see this example on regex101.com for full explanation.