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

🎨 increase timeout in tip e2e test #6568

Merged

Conversation

matusdrobuliak66
Copy link
Contributor

What do these changes do?

  • 🎨 It seems that the new TIP version (2.0.23) is slower during the reporting phase.

Related issue/s

How to test

Dev-ops checklist

@matusdrobuliak66 matusdrobuliak66 self-assigned this Oct 21, 2024
@matusdrobuliak66 matusdrobuliak66 marked this pull request as ready for review October 21, 2024 08:27
@matusdrobuliak66 matusdrobuliak66 added this to the MartinKippenberger milestone Oct 21, 2024
@matusdrobuliak66 matusdrobuliak66 added the e2e Bugs found by or related to the end-2-end testing label Oct 21, 2024
@matusdrobuliak66 matusdrobuliak66 enabled auto-merge (squash) October 21, 2024 08:52
Copy link
Member

@pcrespov pcrespov left a comment

Choose a reason for hiding this comment

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

THOUGHT: would it make sense to include some extra env vars to tune these. Note that the timeouts depend on the machines therefore it would require different values for different CI setups. I propose having a _DELTA,"_FACTOR" env var that can be setup on the CI side.

def get_delta(envname)
   value = int(os.getenv(f"{envname}_DELTA", 0))
   logger.info("%s increased by %s secs", value)
   retun value

def get_factor(envname)
   return int(os.getenv(f"{envname}_FACTOR", 1))
   
   
_JLAB_RUN_OPTIMIZATION_APPEARANCE_TIME: Final[int] = 2 * MINUTE + int(os.getenv("_JLAB_RUN_OPTIMIZATION_APPEARANCE_TIME_DELTA", 0))
_JLAB_RUN_OPTIMIZATION_MAX_TIME: Final[int] = 4 * MINUTE *get_factor("_JLAB_RUN_OPTIMIZATION_MAX_TIME")
_JLAB_REPORTING_MAX_TIME: Final[int] = 60 * SECOND+ get_delta("_JLAB_REPORTING_MAX_TIME")

Alternatively

def get_timeout(envname, default):
   delta = int(os.getenv(f"{envname}_DELTA", 0))
   factor = int(os.getenv(f"{envname}_FACTOR", 1)) 
   value = default * factor + delta
   logger.info("%s", "{envname}={value} ({default=}, {factor=}, {delta=}) ")
   return value

_JLAB_RUN_OPTIMIZATION_APPEARANCE_TIME: Final = get_timeout("_JLAB_RUN_OPTIMIZATION_APPEARANCE_TIME", 2 * MINUTE)

The _JLAB_RUN_OPTIMIZATION_APPEARANCE_TIME and _JLAB_RUN_OPTIMIZATION_APPEARANCE_TIME can live in the CI repository and they are under version control i.e. we can see how we have been changing these in time...

Copy link

codecov bot commented Oct 21, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 64.7%. Comparing base (cafbf96) to head (9ae26c7).
Report is 654 commits behind head on master.

❗ There is a different number of reports uploaded between BASE (cafbf96) and HEAD (9ae26c7). Click for more details.

HEAD has 1 upload less than BASE
Flag BASE (cafbf96) HEAD (9ae26c7)
unittests 1 0
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff            @@
##           master   #6568      +/-   ##
=========================================
- Coverage    84.5%   64.7%   -19.9%     
=========================================
  Files          10     606     +596     
  Lines         214   30747   +30533     
  Branches       25     265     +240     
=========================================
+ Hits          181   19904   +19723     
- Misses         23   10782   +10759     
- Partials       10      61      +51     
Flag Coverage Δ
integrationtests 64.7% <ø> (?)
unittests ?

Flags with carried forward coverage won't be shown. Click here to find out more.

see 616 files with indirect coverage changes

@matusdrobuliak66
Copy link
Contributor Author

matusdrobuliak66 commented Oct 21, 2024

THOUGHT: would it make sense to include some extra env vars to tune these. Note that the timeouts depend on the machines therefore it would require different values for different CI setups. I propose having a _DELTA,"_FACTOR" env var that can be setup on the CI side.

def get_delta(envname)
   value = int(os.getenv(f"{envname}_DELTA", 0))
   logger.info("%s increased by %s secs", value)
   retun value

def get_factor(envname)
   return int(os.getenv(f"{envname}_FACTOR", 1))
   
   
_JLAB_RUN_OPTIMIZATION_APPEARANCE_TIME: Final[int] = 2 * MINUTE + int(os.getenv("_JLAB_RUN_OPTIMIZATION_APPEARANCE_TIME_DELTA", 0))
_JLAB_RUN_OPTIMIZATION_MAX_TIME: Final[int] = 4 * MINUTE *get_factor("_JLAB_RUN_OPTIMIZATION_MAX_TIME")
_JLAB_REPORTING_MAX_TIME: Final[int] = 60 * SECOND+ get_delta("_JLAB_REPORTING_MAX_TIME")

Alternatively

def get_timeout(envname, default):
   delta = int(os.getenv(f"{envname}_DELTA", 0))
   factor = int(os.getenv(f"{envname}_FACTOR", 1)) 
   values = default * factor + delta
     msg= "{envname}={value} ({default=}, {factor=}, {delta=}) "
   logger.info("%s increased by %s secs  (default)", value)
   retun value

_JLAB_RUN_OPTIMIZATION_APPEARANCE_TIME: Final = get_timeout("_JLAB_RUN_OPTIMIZATION_APPEARANCE_TIME", 2 * MINUTE)

Yes, that's one of the options, but on the other hand, it adds extra complexity. It depends on what we want to measure. If we want to measure performance on a specific configuration, say in-house vs AWS, then yes, it makes sense. However, if we're only concerned with checking whether the service is working, I would keep it simple and use just one timeout (the higher of the two), so we don't deviate too much with test configurations.

Copy link

@matusdrobuliak66 matusdrobuliak66 merged commit edc5359 into ITISFoundation:master Oct 21, 2024
57 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
e2e Bugs found by or related to the end-2-end testing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants