-
Notifications
You must be signed in to change notification settings - Fork 15
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
fix #1505 #1508
fix #1505 #1508
Conversation
# Conflicts: # tests/php/View/CalendarTest.php
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #1508 +/- ##
============================================
+ Coverage 48.86% 48.95% +0.09%
- Complexity 2651 2657 +6
============================================
Files 96 96
Lines 10601 10597 -4
============================================
+ Hits 5180 5188 +8
+ Misses 5421 5409 -12 ☔ View full report in Codecov by Sentry. |
# Conflicts: # tests/php/Repository/TimeframeTest.php
Ok, ich werde es zeitnah testen (sorry, ich habe bisher nicht auf dem Schirm gehabt, dass ich angesprochen wurde). Ich frage mich, was am Zeitpunkt 23:59:59 besonders ist. An anderen Stellen im Code ist es 23:59 (also 23:59:00). |
Nein, der Patch hat das beschriebene Problem leider nicht geheilt (Kombination von langem Zeitrahmen und einem einzelnen zusätzlichen Öffnungstag mittendrin - bis zum zusätzlichen Einzel-Öffnungstag kann man leider aus dem langem Zeitrahmen nichts buchen). |
@nelarsen Kannst du mir mal zwei Beispielzeiträume dafür geben damit ich dafür einen Testcase schreiben kann? |
@nelarsen Danke dir, das wäre bestimmt nicht schlecht. Aber vielleicht sollte ich am besten sowieso diesen Test refactoren mit einem DataProvider für die möglichen Kombinationen. Das wird jetzt schon unübersichtlich. |
Ok, ich warte auf weitere Anweiseungen (-: |
@nelarsen Ich habe die Tests refactored und dabei ist mir auch noch ein Bug aufgefallen. Kannst du vielleicht deinen Testcase in den dataProvider provideGetClosestBookableTimeFrameForToday einfügen? |
@nelarsen Habe mal versucht den Testcase nachzustellen, bei mir läuft das durch. Habe ich was vergessen? |
Ich glaube nicht. Aber ich glaube, dass du hast du Fehler behoben. 🥳 Jedenfalls habe ich noch mal manuell getestet. Zuerst der alte Stand (sha d528a8d) und dann der neue Stand (sha 98e1679). Das war ausschlagggebend, jetzt funktioniert es. Irgendwo dazwischen hast du wohl den Fehler behoben, kann es sein? |
Super, das freut mir. Ich hatte einen Operator falschrum gesetzt |
@nelarsen Kannst du beizeiten mal schauen, ob der Patch für dich funktioniert?
closes #1505