Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Development: Reduce random data in server tests #6602

Merged
merged 6 commits into from
May 27, 2023

Conversation

DominikRemo
Copy link
Contributor

@DominikRemo DominikRemo commented May 19, 2023

Checklist

General

Server

Motivation and Context

Random test data can lead to flaky tests. Furthermore, generating random data for tests creates an unnecessary overhead compared to hard coded values.

Description

In this PR, the majority of random test data has been replaced with either hard coded values, or a combination of hard coded values and unique identifiers such as IDs.

Steps for Testing

Test changes only

Review Progress

Code Review

  • Code Review 1
  • Code Review 2

Copy link
Contributor

@MichaelOwenDyer MichaelOwenDyer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A small change for the better

Artemis Development automation moved this from In progress to Review in progress May 24, 2023
@artemis-bot artemis-bot moved this from Review in progress to In progress in Artemis Development May 24, 2023
@ls1intum ls1intum deleted a comment from codecov-commenter May 24, 2023
@DominikRemo DominikRemo marked this pull request as ready for review May 24, 2023 15:34
@DominikRemo DominikRemo requested a review from a team as a code owner May 24, 2023 15:34
@artemis-bot artemis-bot moved this from In progress to Ready for review in Artemis Development May 24, 2023
terlan98
terlan98 previously approved these changes May 25, 2023
Copy link
Contributor

@terlan98 terlan98 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The changes look good. Added a suggestion to reduce random data even further.

laadvo
laadvo previously approved these changes May 25, 2023
Copy link
Contributor

@laadvo laadvo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

changes look good to me 👍

Copy link
Contributor

@MaximilianAnzinger MaximilianAnzinger left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me 👍

Artemis Development automation moved this from Ready for review to Review in progress May 26, 2023
@github-actions github-actions bot added the client Pull requests that update TypeScript code. (Added Automatically!) label May 26, 2023
@github-actions github-actions bot added cypress Pull requests that update cypress tests. (Added Automatically!) database Pull requests that update the database. (Added Automatically!). Require a CRITICAL deployment. server Pull requests that update Java code. (Added Automatically!) labels May 26, 2023
@github-actions github-actions bot removed cypress Pull requests that update cypress tests. (Added Automatically!) client Pull requests that update TypeScript code. (Added Automatically!) server Pull requests that update Java code. (Added Automatically!) database Pull requests that update the database. (Added Automatically!). Require a CRITICAL deployment. labels May 26, 2023
Copy link
Contributor

@laadvo laadvo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

reapprove after merge conflict

@codecov-commenter
Copy link

Codecov Report

Merging #6602 (7299749) into develop (274368b) will increase coverage by 0.00%.
The diff coverage is n/a.

❗ Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more.

Additional details and impacted files
@@            Coverage Diff             @@
##             develop    #6602   +/-   ##
==========================================
  Coverage      80.39%   80.39%           
- Complexity     13588    13589    +1     
==========================================
  Files           2398     2398           
  Lines          91683    91683           
  Branches       12870    12870           
==========================================
+ Hits           73704    73709    +5     
+ Misses          9889     9880    -9     
- Partials        8090     8094    +4     

see 9 files with indirect coverage changes

Flag Coverage Δ
client 76.13% <ø> (-0.01%) ⬇️
server 85.06% <ø> (+0.01%) ⬆️

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 274368b...7299749. Read the comment docs.

Copy link
Contributor

@terlan98 terlan98 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reapproved

Copy link
Contributor

@MaximilianAnzinger MaximilianAnzinger left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reapproved after merge

@krusche krusche merged commit f2a3c3e into develop May 27, 2023
24 of 27 checks passed
@krusche krusche deleted the test/reduce-random-test-data branch May 27, 2023 14:19
@krusche krusche added this to the 6.2.0 milestone May 27, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
No open projects
Artemis Development
  
Review in progress
Development

Successfully merging this pull request may close these issues.

None yet

7 participants