You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@drill.apache.org by "jnturton (via GitHub)" <gi...@apache.org> on 2023/02/14 11:52:02 UTC
[GitHub] [drill] jnturton opened a new pull request, #2756: DRILL-8117: Upgrade unit tests to the cluster fixture framework
jnturton opened a new pull request, #2756:
URL: https://github.com/apache/drill/pull/2756
# [DRILL-8117](https://issues.apache.org/jira/browse/DRILL-8117): Upgrade unit tests to the cluster fixture framework
## Description
Upgrades various unit tests to the cluster fixture framework and replaces other instances of deprecated code usage.
Methods client.alterSession() and client.alterSystem() run sql silently which would not throw exception, so it doesn't match the purpose for some test cases. And I still keep using run() here for consistence.
For `TestInboundImpersonation`
Because cluster fixture save client properties even client closed, use a dedicated cluster fixture for each test case to ensure the client fixture has clean properties.
For `TestImpersonationMetadata`
Only use one cluster from test start to the end will induce buffer overflow. So change BeforeClass to Before to start a new cluster for each test case.
## Documentation
N/A
## Testing
Existing unit tests.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@drill.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [drill] jnturton commented on pull request #2756: DRILL-8117: Upgrade unit tests to the cluster fixture framework
Posted by "jnturton (via GitHub)" <gi...@apache.org>.
jnturton commented on PR #2756:
URL: https://github.com/apache/drill/pull/2756#issuecomment-1430114758
> One other question. Should we document this in the developer documentation?
I think we do have developer documentation describing cluster tests, or do you mean something else?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@drill.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [drill] jnturton commented on pull request #2756: DRILL-8117: Upgrade unit tests to the cluster fixture framework
Posted by "jnturton (via GitHub)" <gi...@apache.org>.
jnturton commented on PR #2756:
URL: https://github.com/apache/drill/pull/2756#issuecomment-1430704625
Okay, I think the penny's finally dropped. I was also thinking about the markdown in /docs but couldn't fathom what we'd add. But the new UserExceptionMatcher usage can be described and also the try-with-resources style of creating single-use client fixtures.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@drill.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [drill] cgivre merged pull request #2756: DRILL-8117: Upgrade unit tests to the cluster fixture framework
Posted by "cgivre (via GitHub)" <gi...@apache.org>.
cgivre merged PR #2756:
URL: https://github.com/apache/drill/pull/2756
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@drill.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [drill] cgivre commented on pull request #2756: DRILL-8117: Upgrade unit tests to the cluster fixture framework
Posted by "cgivre (via GitHub)" <gi...@apache.org>.
cgivre commented on PR #2756:
URL: https://github.com/apache/drill/pull/2756#issuecomment-1429699470
@jnturton @kingswanwho Should we close the other PR?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@drill.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [drill] cgivre commented on pull request #2756: DRILL-8117: Upgrade unit tests to the cluster fixture framework
Posted by "cgivre (via GitHub)" <gi...@apache.org>.
cgivre commented on PR #2756:
URL: https://github.com/apache/drill/pull/2756#issuecomment-1429701817
One other question. Should we document this in the developer documentation?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@drill.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [drill] kingswanwho commented on pull request #2756: DRILL-8117: Upgrade unit tests to the cluster fixture framework
Posted by "kingswanwho (via GitHub)" <gi...@apache.org>.
kingswanwho commented on PR #2756:
URL: https://github.com/apache/drill/pull/2756#issuecomment-1430637603
> > > One other question. Should we document this in the developer documentation?
> >
> >
> > I think we do have developer documentation describing cluster fixture tests, or do you mean something else?
>
> I was referring to the markdown files in the `/docs` folder. With this PR do those need to be updated? (It doesn't have to be a part of this PR.)
Hi Charles, I have checked /docs developer information, this PR transfer test framework from BaseTestQuery to ClusterTest, and doesn't change the test logic of ClusterTest. James helps to find a clean way to create new ClientFixture, and handle UserException. I can help to update those information in /docs in a new PR.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@drill.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [drill] cgivre commented on pull request #2756: DRILL-8117: Upgrade unit tests to the cluster fixture framework
Posted by "cgivre (via GitHub)" <gi...@apache.org>.
cgivre commented on PR #2756:
URL: https://github.com/apache/drill/pull/2756#issuecomment-1430116439
> > One other question. Should we document this in the developer documentation?
>
> I think we do have developer documentation describing cluster fixture tests, or do you mean something else?
I was referring to the markdown files in the `/docs` folder. With this PR do those need to be updated? (It doesn't have to be a part of this PR.)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@drill.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [drill] jnturton commented on pull request #2756: DRILL-8117: Upgrade unit tests to the cluster fixture framework
Posted by "jnturton (via GitHub)" <gi...@apache.org>.
jnturton commented on PR #2756:
URL: https://github.com/apache/drill/pull/2756#issuecomment-1430707391
Message to whoever squashes and merges here, in case it's not me: when cleaning up the squashed commit detail message please retain the co-author footer so that the repo will reflect @kingswanwho's contribution.
```
---------
Co-authored-by: kingswanwho <ji...@u.northwestern.edu>
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@drill.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [drill] kingswanwho commented on pull request #2756: DRILL-8117: Upgrade unit tests to the cluster fixture framework
Posted by "kingswanwho (via GitHub)" <gi...@apache.org>.
kingswanwho commented on PR #2756:
URL: https://github.com/apache/drill/pull/2756#issuecomment-1430077393
> @jnturton @kingswanwho Should we close the other PR?
Yes, I closed another PR
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@drill.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org