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

Fix hard-coded timestamps in main.derived_condition_pushdown MTR test #469

Conversation

dlenski
Copy link

@dlenski dlenski commented May 24, 2023

Two hard-coded timestamps were added to this test in 22f6ef90cae8#diff-d7e1b8c7138b94f92fbd555c44e7facc027cb50217f59f51729444ee7c3b37a4R1950

  1. '2022-05-06 16:49:45' (assumed by the test's result file to be < NOW())
  2. '2023-05-06 16:49:45' (assumed by the test's result file to be >= NOW())

Because the second of these timestamps is now in the past, the test is now failing due to result mismatch.

Tests should not be written to include hard-coded assumptions about the local time in their execution environment; these are essentially guaranteed to get out-of-date and cause failures.

This test can very easily be rewritten to choose past/future timestamps relative to the time at which it is run,
DATE_ADD(NOW(), INTERVAL [+-] 1 DAY).

Note that Ubuntu's build environment had to disable this test on 2022-05-09, likely due to encountering this problem:
https://bugs.launchpad.net/ubuntu/+source/mysql-8.0/8.0.33-0ubuntu2

Two hard-coded timestamps were added to this test in
mysql@22f6ef90cae8#diff-d7e1b8c7138b94f92fbd555c44e7facc027cb50217f59f51729444ee7c3b37a4R1950

1. '2022-05-06 16:49:45' (assumed by the test's result file to be `< NOW()`)
2. '2023-05-06 16:49:45' (assumed by the test's result file to be `>= NOW()`)

Because the second of these timestamps is now in the past, the test is now
failing due to result mismatch.

Tests should not be written to include hard-coded assumptions about the
local time in their execution environment; these are essentially guaranteed
to get out-of-date and cause failures.

This test can very easily be rewritten to choose past/future timestamps
relative to the time at which it is run,
`DATE_ADD(NOW(), INTERVAL [+-] 1 DAY)`.

Note that Ubuntu's build environment had to disable this test on 2022-05-09,
likely due to encountering this problem:

    https://bugs.launchpad.net/ubuntu/+source/mysql-8.0/8.0.33-0ubuntu2

All new code of the whole pull request, including one or several files that are
either new files or modified ones, are contributed under the BSD-new license. I
am contributing on behalf of my employer Amazon Web Services, Inc.
@mysql-oca-bot
Copy link

Hi, thank you for submitting this pull request. In order to consider your code we need you to sign the Oracle Contribution Agreement (OCA). Please review the details and follow the instructions at https://oca.opensource.oracle.com/
Please make sure to include your MySQL bug system user (email) in the returned form.
Thanks

@dlenski
Copy link
Author

dlenski commented May 24, 2023

I informed the Debian mysql packagers about this as well: https://salsa.debian.org/mariadb-team/mysql/-/merge_requests/67#note_400519

… and the Ubuntu packagers: https://bugs.launchpad.net/ubuntu/+source/mysql-8.0/+bug/2020696

@dlenski
Copy link
Author

dlenski commented May 31, 2023

Could any of the MySQL maintainers please give some initial feedback on this submission?

Are you happy with this approach to fixing the test? Do you want me to continue with it?

@mysql-admin
Copy link

Hi @dlenski,
We cannot process or review the contribution before an you sign the OCA and confirm the contribution is under the terms of the OCA (as requested above from the bot).

Thanks for contributing to MySQL
==Omer

@dlenski
Copy link
Author

dlenski commented Jun 16, 2023

This contribution is under the OCA signed by Amazon and covering submissions to the MySQL project.

@mysql-oca-bot
Copy link

Hi, there was no response to our request to sign an OCA or confirm the code is submitted under the terms of the OCA. As such this request will be closed.
Thanks

@dlenski
Copy link
Author

dlenski commented Jun 27, 2023

Hi, there was no response to our request to sign an OCA or confirm the code is submitted under the terms of the OCA.

This is not correct. Why was this closed?

The comment immediately above confirms that an OCA has been signed:

image

@mysql-admin
Copy link

H @dlenski,
As discussed off line there was a missing match between the OCA and your account as such the PR was closed.
Re-opening. You will soon get a message from the bot asking you to confirm the request s under the OCA
Sorry for the difficulties encountered and thanks for contributing to MySQL
==Omer

@mysql-admin mysql-admin reopened this Jun 28, 2023
@mysql-oca-bot
Copy link

Hi, thank you for your contribution. Please confirm this code is submitted under the terms of the OCA (Oracle's Contribution Agreement) you have previously signed by cutting and pasting the following text as a comment:
"I confirm the code being submitted is offered under the terms of the OCA, and that I am authorized to contribute it."
Thanks

@mysql-oca-bot
Copy link

Hi, thank you for your contribution. Your code has been assigned to an internal queue. Please follow
bug http://bugs.mysql.com/bug.php?id=111606 for updates.
Thanks

@athos-ribeiro
Copy link

Has this been merged yet?

@dlenski
Copy link
Author

dlenski commented Nov 29, 2023

@athos-ribeiro, it appears not. Apparently an "alternative fix" was applied internal to Oracle/MySQL… but, as I quickly pointed out, that "fix" does not actually fix the problem. 🤦‍♂️

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants