Skip to content

#1568 Remove test-specific delay from prod code#1618

Merged
tomholub merged 14 commits intomasterfrom
bugfix/issue-1568-remove-timeout
May 19, 2022
Merged

#1568 Remove test-specific delay from prod code#1618
tomholub merged 14 commits intomasterfrom
bugfix/issue-1568-remove-timeout

Conversation

@sosnovsky
Copy link
Collaborator

@sosnovsky sosnovsky commented May 17, 2022

This PR removes test-specific delay from prod code.

close #1568


Tests (delete all except exactly one):

  • Does not need tests (refactor only, docs or internal changes)

To be filled by reviewers

I have reviewed that this PR... (tick whichever items you personally focused on during this review):

  • addresses the issue it closes (if any)
  • code is readable and understandable
  • is accompanied with tests, or tests are not needed
  • is free of vulnerabilities
  • is documented clearly and usefully, or doesn't need documentation

tomholub
tomholub previously approved these changes May 17, 2022
sosnovsky added 2 commits May 17, 2022 15:45
# Conflicts:
#	FlowCrypt/Functionality/Services/EKMVcHelper.swift
@sosnovsky sosnovsky marked this pull request as ready for review May 18, 2022 08:07
@sosnovsky sosnovsky requested review from ioanmo226 and tomholub May 18, 2022 08:07
@sosnovsky sosnovsky enabled auto-merge (squash) May 18, 2022 08:07
@sosnovsky
Copy link
Collaborator Author

@sosnovsky As I thought, this issue still happens. Please check and fix

Re-checking it

@sosnovsky sosnovsky marked this pull request as draft May 18, 2022 13:53
auto-merge was automatically disabled May 18, 2022 13:53

Pull request was converted to draft

@sosnovsky sosnovsky marked this pull request as ready for review May 19, 2022 10:40
@sosnovsky
Copy link
Collaborator Author

@ioanmo226 I tried to run tests a couple times on semaphore - they passed well, seems like issue is fixed now.
I added timeout before app restarting, maybe on semaphore app termination took more time and lead to crashes.

@tomholub tomholub merged commit 4d3d4e4 into master May 19, 2022
@tomholub tomholub deleted the bugfix/issue-1568-remove-timeout branch May 19, 2022 11:01
@sosnovsky
Copy link
Collaborator Author

@sosnovsky This issue happened again on master.

@ioanmo226 Looks like this test failed even with previously added delay, for example here - https://flowcrypt.semaphoreci.com/workflows/64b634cc-e384-4a9d-85ae-9347c0776396?pipeline_id=45cea1b4-0b99-4b84-a184-aa73f6a8e4b3

Seems to be not related for changes in this PR, so I created another issue for it - #1631

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

avoid test-specific delays in prod code

2 participants