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.

Sign up for GitHub

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

Jump to bottom

Move update-tester tests to markdown files #522

Merged
merged 1 commit into from
Sep 16, 2022
Merged

Move update-tester tests to markdown files #522

merged 1 commit into from
Sep 16, 2022

Conversation

thatzopoulos
Copy link
Contributor

@thatzopoulos thatzopoulos commented Sep 6, 2022 •
edited
Loading

Changes to the update-tester:

  • Hardcoded tests are now read in from markdown files, similar to sql-doctester
  • Update-tester will now set all timezones to UTC before running sql scripts
  • Ability to set a min_toolkit_version that tests can run on

@thatzopoulos thatzopoulos changed the title moved hardcoded update-tester tests to markdown files Move update-tester tests to markdown files Sep 6, 2022
epgts
epgts reviewed Sep 6, 2022
View reviewed changes
Copy link
Contributor

@epgts epgts left a comment

Choose a reason for hiding this comment

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

I haven't read the Rust yet.

sql/original_update_tests.md Outdated Show resolved Hide resolved
sql/original_update_tests.md Outdated Show resolved Hide resolved
sql/original_update_tests.md Outdated Show resolved Hide resolved
sql/original_update_tests.md Outdated Show resolved Hide resolved
sql/original_update_tests.md Outdated Show resolved Hide resolved
tools/update-tester/src/parser.rs Show resolved Hide resolved
@thatzopoulos thatzopoulos force-pushed the th/update-tester-test-reformat branch 2 times, most recently from ac08234 to 3880272 Compare September 8, 2022 21:28
@thatzopoulos thatzopoulos marked this pull request as ready for review September 9, 2022 19:31
@thatzopoulos thatzopoulos force-pushed the th/update-tester-test-reformat branch 5 times, most recently from 02a2013 to 87c240d Compare September 12, 2022 21:06
epgts
epgts reviewed Sep 13, 2022
View reviewed changes
Copy link
Contributor

@epgts epgts left a comment

Choose a reason for hiding this comment

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

Most of these (as discussed) are notes for the future. I think this is basically ready to go.

Thanks!

sql/original_update_tests.md Outdated Show resolved Hide resolved
tools/update-tester/src/main.rs Show resolved Hide resolved
tools/update-tester/src/main.rs Show resolved Hide resolved
tools/update-tester/src/parser.rs Outdated Show resolved Hide resolved
tools/update-tester/src/parser.rs Outdated Show resolved Hide resolved
tools/update-tester/src/testrunner.rs
) -> Vec<(Test, Result<(), TestError>)> {
let all_tests = parser::extract_tests("sql");
// Hack to match previous versions of toolkit that don't conform to Semver.
let current_toolkit_semver = match current_toolkit_version.as_str() {
Copy link
Contributor

Choose a reason for hiding this comment

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

[After live discussion, we think I was confused when I wrote this, because actually we want the tests even for old versions to come FROM THE NEW CODE. Not only would running old create-test-objects be pointless, but running new create-test-objects lets us add new tests for old versions.
So ignore what's below for now.
Recording for posterity.]

Is this necessary? Two questions make me think it may not be:

  1. I want to drop all releases older than 1.9 from upgradeable_from for 1.12 and all older than 1.10 for 1.13.
  2. Will we be able to run 1.11's validate-test-objects against 1.10's create-test? If not, s/1.10/1.11/ in Approximate Percentile #1 :)

I don't think I've spoken of my push to drop <1.9 from upgradeable_from so let me get down an early draft here:

We can't create-test-objects on any release older than 1.10, which means our current test automation plan won't support testing upgrading from any release older than that without having two automation schemes at twice the price (of both build AND maintain!), which means we can't call "release automation" done until we drop the old versions.

tools/update-tester/src/testrunner.rs Show resolved Hide resolved
tools/update-tester/src/main.rs Outdated Show resolved Hide resolved
@thatzopoulos thatzopoulos force-pushed the th/update-tester-test-reformat branch 2 times, most recently from d6884f7 to 2df7306 Compare September 15, 2022 18:04
epgts
epgts approved these changes Sep 15, 2022
View reviewed changes
Copy link
Contributor

@epgts epgts left a comment

Choose a reason for hiding this comment

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

one last question: is it all rustfmted? :)

@thatzopoulos
Copy link
Contributor Author

It seems to all be rustfmted for me locally

@rtwalker
Copy link
Contributor

It seems to all be rustfmted for me locally

same!

~/t/timescaledb-toolkit (th/update-tester-test-reformat)> cargo fmt --check
~/t/timescaledb-toolkit (th/update-tester-test-reformat)>

@thatzopoulos
Copy link
Contributor Author

bors r+

@bors
Copy link
Contributor

bors bot commented Sep 16, 2022

Build succeeded:

@bors bors bot merged commit 26fc53a into main Sep 16, 2022
@bors bors bot deleted the th/update-tester-test-reformat branch September 16, 2022 16:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@epgts epgts epgts approved these changes

@WireBaron WireBaron Awaiting requested review from WireBaron

@syvb syvb Awaiting requested review from syvb

@rtwalker rtwalker Awaiting requested review from rtwalker

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@thatzopoulos @rtwalker @epgts