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

ccl/sqlproxyccl: broken tests in Bazel #61915

Closed
rickystewart opened this issue Mar 12, 2021 · 2 comments · Fixed by #64441
Closed

ccl/sqlproxyccl: broken tests in Bazel #61915

rickystewart opened this issue Mar 12, 2021 · 2 comments · Fixed by #64441
Assignees
Labels
C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior.

Comments

@rickystewart
Copy link
Collaborator

rickystewart commented Mar 12, 2021

We are making a lot of progress in the migration from make to Bazel. Before we can complete our migration, we need to make sure that all tests in-tree continue to pass when run inside of the Bazel sandbox. The following tests are broken when run in the Bazel sandbox:

pkg/ccl/sqlproxyccl:sqlproxyccl_test

Please help us by doing the following:

  • Reproduce the failure locally; for example, with bazel test bazel test pkg/ccl/sqlproxyccl:sqlproxyccl_test, and consult the test logs to see what went wrong
  • Identify the root cause of the failure
  • If you are able, repair the failure such that the test passes, and submit a PR removing the broken_in_bazel tag from the test (see the linked documentation below)
  • If not, take your findings to #bazel in Slack, and we’ll work with you to figure out how we can get the tests working

Helpful documentation:

@rickystewart rickystewart added the C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. label Mar 12, 2021
@rickystewart
Copy link
Collaborator Author

Just checked on this -- this is still broken. Failures look like this:

=== RUN   TestFrontendAdmitWithClientSSLRequire
    frontend_admitter_test.go:90: 
        	Error Trace:	frontend_admitter_test.go:90
        	Error:      	Received unexpected error:
        	            	open testserver.crt: no such file or directory
        	Test:       	TestFrontendAdmitWithClientSSLRequire

Presumably the tests need to add testserver{.crt,.key,_config.cnf} to their testdata, and then make sure they're accessing those files in the proper way.

@andy-kimball andy-kimball assigned darinpp and unassigned andy-kimball Apr 29, 2021
@andy-kimball
Copy link
Contributor

Darin, can you take a look at this?

craig bot pushed a commit that referenced this issue Apr 30, 2021
64306: pgcode: remove a couple of deprecated errors r=yuzefovich a=yuzefovich

Release note: None

64441: ccl/sqlproxyccl: fix a broken test in bazel r=darinpp a=darinpp

This fixes #61915
by adding the required data files.

Release note: None

64444: opt: do not provide project ordering for columns removed when simplified r=mgartner a=mgartner

Previously, `projectCanProvideOrdering` did not simplify an ordering
base on an FD set if the original ordering could be provided, while
`projectBuildChildReqOrdering` always attempted to simplify an ordering.
In #64342 the behavior of `OrderingChoice.Simplify` changed  to remove
columns in groups that are not known to be equivalent by the FD. As a
result, `projectCanProvideOrdering` could return true in cases where the
simplified ordering, with some columns removed, could not be provided,
causing `ordering.projectBuildChildReqOrdering` to panic.

This commit updates `ordering.projectCanProvideOrdering` so that the
ordering is always simplified, matching the behavior of
`ordering.projectBuildChildReqOrdering`.

Fixes #64399

Release note: None

64483: bazel: prefer `exec_tools` to `tools` in `BUILD` files r=rail a=rickystewart

The [documentation](https://docs.bazel.build/versions/master/be/general.html#genrule_args)
specifies that `exec_tools` is preferred to `tools` wherever possible.
It doesn't matter in our case, so let's just standardize on `exec_tools`
everywhere.

Release note: None

Co-authored-by: Yahor Yuzefovich <[email protected]>
Co-authored-by: Darin Peshev <[email protected]>
Co-authored-by: Marcus Gartner <[email protected]>
Co-authored-by: Ricky Stewart <[email protected]>
@craig craig bot closed this as completed in 1176126 Apr 30, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants