search

dbcli / mssql-cli

A command-line client for SQL Server with auto-completion and syntax highlighting
BSD 3-Clause "New" or "Revised" License
1.35k stars 191 forks source link

improved debugging and pep 8 support for globalization tests #335

Closed ellbosch closed 4 years ago

ellbosch commented 4 years ago

Our Windows build recently failed during one of our globalization tests. This is a known flaky test--however, there isn't any useful stack trace to help us debug the failures due to generic exception handling.

Therefore, I removed generic exception handling so that errors are no longer suppressed. Furthermore, I used this opportunity to convert unit tests to Pytest (a more modern version of Python's built-in unit test module) as well as add PEP 8 compatibility.

Tags: #281, #283

ellbosch commented 4 years ago

Looks like I was handling cleanup incorrectly with Python 2 versions--will commit a fix here.

ellbosch commented 4 years ago

Got the error again, this time in Linux. Last time it was Windows, so there doesn't appear to be an OS-related issue here.

The special package occasionally throws an error saying the command isn't found for a CREATE SCHEMA query. I've only seen this happen with the globalizations tests, so I'm guessing it may have something to do with that. Furthermore, the only test I've seen fail is the first one in the tests for result set.

I'm unsure why this happens only some of the time. With my last commit, we'll have more info on the parsed command that isn't getting found.

ellbosch commented 4 years ago

This looks OK in terms of implementation. 2 things for the future:

  • Please separate out change to test framework from the specific code changes to identify problem. Combining makes it hard to track
  • I'm wondering whether changes like this make us move away from what the other dbcli tools use for code, framework etc. Are we trying to align with them in any way? Can we build on them in terms of test reliability, what tests to run, etc.?

I'd prefer if we split this into the 2 PRs just so they're clean, but OK with this as-is.

Fair point on splitting this up into different PRs--I definitely got a little carried away here. I'll make sure to do that in the future.

Alignment with other dbcli's is something we can do better. I can reach out to the folks to see what they do.