-
Notifications
You must be signed in to change notification settings - Fork 157
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 acceptance tests with new v-calendar #8151
Comments
@amrita-shrestha @SagarGi please get someone to sort this out. |
We made the OcDatepicker component instance available on the DOM Element to be able to fix the As it's used in one acceptance test only, it shouldn't be too much of a performance problem if we just click in the UI I hope. Moreover we fixed the infamous December issue recently, which should be included in the v calendar version used in the PR, so we can get rid of that extra code path as well |
There's also an e2e Test which uses the same more or less internal api, up to you but it might be worth porting the acceptance test over and fixing it just once |
The problem with the failing acceptance test The proper fix for the date picker tests would be to fix the auto-scrolling issue of the calendar CC @dschmidt |
Changing the test code will only make the tests pass for a certain period of time. |
I will post the current (master) and new (vue3-compat-ng) scrolling behavior of the calendar. |
Ok, cool. If it's just a test issue I'd be happy if we could get a temporary fix which won't make the scope of the PR much bigger, as it's really hard to review already |
Let's fix it properly in a follow up maybe? |
I think the behavior is the same on both. The only difference is the calendar size due to which the test is passing on master but not in Master: simplescreenrecorder-2022-12-27_18.08.42.mp4vue3-compat-ng: simplescreenrecorder-2022-12-27_18.06.35.mp4
Maybe we can adjust the date as of now. |
But we also want to be sure that the issue with auto-scroll is caught |
yes, adjust the future date to some closer date that does not cause this known date-picker problem. The test scenario is just testing that the expiration date of a public link share can be adjusted. The dates currently chosen are after the date for the https://en.wikipedia.org/wiki/Year_2038_problem and so they are checking that the software does not have this future problem. But I don't think that we need to be doing that specifically in a test like this. We can easily add specific "year 2038" test scenarios as a separate thing some time in the future. |
Can this be closed now? |
Yes - closing. The "year 2038" is a different thing, and we are some way off from rushing to test that everywhere just yet! |
In #8128 we updated to a new major version of v-calendar and while it seems to work just fine when testing it manually, the acceptance tests are broken - so we skipped them for now.
It would be great if the QA Team could properly fix it.
Thanks in advance!
See commit "Skip expiration date picker acceptance test as it's broken with the n…" - currently it is 932c408 here but we are rebasing constantly, so possibly not for long.
The text was updated successfully, but these errors were encountered: