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

Run SQL server tests on Azure SQL Edge #714

Merged
merged 1 commit into from Mar 17, 2022

Conversation

jfhbrook-at-work
Copy link
Contributor

@jfhbrook-at-work jfhbrook-at-work commented Mar 15, 2022

While taking a look at floating #666 on an internal fork, I learned that the mssql images are x86 only. This means that if you run the tests on an m1 mac today, a bunch of them will fail.

Microsoft is a long way from releasing an arm build of SQL Server, so we can't really wait on it. However, they've released an IOT flavor of SQL Server called Azure SQL Edge which does have arm builds (fun fact, it can run on a raspberry pi) and which is in many cases a drop-in replacement.

tl;dr: This PR adds this image to the tested specs in sqlserver_test.go and only adds the existing mssql images if the runtime architecture is x64-compatible.

Note, I did some slightly funky things with setting up the edge container spec and the port lookup. The current logic assumes that there's only one exposed port on any given image and grabs "the first one" in multiple cases. Unlike the mssql images, the edge image exposes a handful of other ports that coincidentally get listed prior to the port we actually want - hence, manually setting the port to the one we expect and manually passing a port map to the edge options.

I have mssql tests with edge passing locally, and I have the full mssql test suite passing in our org's fork. However, the CI tests aren't passing here. I believe these failures are due to other databases and I consider fixing those out-of-scope for this change, so I haven't attempted to fix these non-mssql tests.

@jfhbrook-at-work jfhbrook-at-work force-pushed the josh/test-sqlserver-with-edge branch 3 times, most recently from 95eebc7 to 72c7343 Compare March 16, 2022 17:39
@coveralls
Copy link

Pull Request Test Coverage Report for Build 1995047444

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 57.807%

Totals Coverage Status
Change from base Build 1895253585: 0.0%
Covered Lines: 3743
Relevant Lines: 6475

💛 - Coveralls

Copy link
Member

@dhui dhui left a comment

Choose a reason for hiding this comment

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

Thanks for the PR!

@dhui dhui merged commit cc5f8b7 into golang-migrate:master Mar 17, 2022
@jfhbrook-at-work
Copy link
Contributor Author

Any time! While I'm here, is there something blocking #666 ? I'm working to float and test it in production and I'd rather get it into a mainline release than mess with a fork! Happy to pitch in.

@jfhbrook-at-work jfhbrook-at-work deleted the josh/test-sqlserver-with-edge branch March 17, 2022 17:07
@dhui
Copy link
Member

dhui commented Mar 17, 2022

Just reviewed #666 . It'll need to pass tests to be merged.

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.

None yet

3 participants