You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@jmeter.apache.org by vlsi <gi...@git.apache.org> on 2016/04/03 02:17:45 UTC
[GitHub] jmeter pull request: Use explicit timeout for TestDNSCacheManager,...
GitHub user vlsi opened a pull request:
https://github.com/apache/jmeter/pull/176
Use explicit timeout for TestDNSCacheManager, so test is executed faster
`org.apache.jmeter.protocol.http.control.TestDNSCacheManager#testCloneWithCustomResolverAndInvalidNameserver` took 30 seconds to execute.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/vlsi/jmeter dns_timeout
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/jmeter/pull/176.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #176
----
commit a2e17f8061a8916fb751f6d5a908165bd424d6b1
Author: Vladimir Sitnikov <si...@gmail.com>
Date: 2016-04-03T00:15:56Z
Use explicit timeout for TestDNSCacheManager, so test is executed faster
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
Re: [GitHub] jmeter pull request: Use explicit timeout for TestDNSCacheManager,...
Posted by Vladimir Sitnikov <si...@gmail.com>.
>So does that mean that PRs are mutable?
Of course they are.
Note:
1) If you access PR via git interface, you have immediate visibility
that PR is updated.
There's no way to mutate the contents of a commit. Its Id would have
to be updated.
2) In github.com UI, commits/comments are ordered by date, so it is
obvious when commit
was created afterwards.
Just go ahead and check that my commit is below Philippe's comment:
https://github.com/apache/jmeter/pull/176#issuecomment-204965711
Note: each commit is immutable, however one can update the PR with
_another_ set of commits.
That is useful for:
1) Corrections for code review
2) Merging multiple commits into a single one.
For instance, Philippe's https://github.com/apache/jmeter/pull/179
consists of 7 commits,
however I do not see much value in publishing 7 "work-in-progress" commits.
Well, it might make sense if you publish one, then another (so other
can see the diff),
but in general single commit per feature makes more sense for me.
Vladimir
Re: [GitHub] jmeter pull request: Use explicit timeout for TestDNSCacheManager,...
Posted by sebb <se...@gmail.com>.
On 3 April 2016 at 14:38, vlsi <gi...@git.apache.org> wrote:
> Github user vlsi commented on the pull request:
>
> https://github.com/apache/jmeter/pull/176#issuecomment-204978579
>
> > Sorry , I had some hallucination although I checked twice :-) , it is package protected.
>
> You are fine. I've just updated the PR as you suggested package-private.
>
So does that mean that PRs are mutable?
That seems like a recipe for confusion (as we have just seen) and
potentially worse.
> ---
> If your project is set up for it, you can reply to this email and have your
> reply appear on GitHub as well. If your project does not have this feature
> enabled and wishes so, or if the feature is enabled but not working, please
> contact infrastructure at infrastructure@apache.org or file a JIRA ticket
> with INFRA.
> ---
[GitHub] jmeter pull request: Use explicit timeout for TestDNSCacheManager,...
Posted by vlsi <gi...@git.apache.org>.
Github user vlsi commented on the pull request:
https://github.com/apache/jmeter/pull/176#issuecomment-204978579
> Sorry , I had some hallucination although I checked twice :-) , it is package protected.
You are fine. I've just updated the PR as you suggested package-private.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] jmeter pull request: Use explicit timeout for TestDNSCacheManager,...
Posted by pmouawad <gi...@git.apache.org>.
Github user pmouawad commented on the pull request:
https://github.com/apache/jmeter/pull/176#issuecomment-204978544
+1 for me
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] jmeter pull request: Use explicit timeout for TestDNSCacheManager,...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/jmeter/pull/176
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] jmeter pull request: Use explicit timeout for TestDNSCacheManager,...
Posted by pmouawad <gi...@git.apache.org>.
Github user pmouawad commented on the pull request:
https://github.com/apache/jmeter/pull/176#issuecomment-204978538
Sorry , I had some hallucination although I checked twice :-) , it is package protected.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
Re: [GitHub] jmeter pull request: Use explicit timeout for TestDNSCacheManager,...
Posted by sebb <se...@gmail.com>.
On 3 April 2016 at 13:48, pmouawad <gi...@git.apache.org> wrote:
> Github user pmouawad commented on the pull request:
>
> https://github.com/apache/jmeter/pull/176#issuecomment-204965711
>
> Hi Vladimir,
> Exposing public method for testing might introduce issues.
> Users can think that field is persisted in JMX while it's not.
>
> Can't it be package protected ?
>
Huh?
Looks to me as though it is package protected.
But it would be good to add a comment to say it is only for unit tests.
Not sure the Javadoc is needed for package methods; it may suggest
these methods is supported, so I would replace it with something like:
// for unit testing purposes only
> ---
> If your project is set up for it, you can reply to this email and have your
> reply appear on GitHub as well. If your project does not have this feature
> enabled and wishes so, or if the feature is enabled but not working, please
> contact infrastructure at infrastructure@apache.org or file a JIRA ticket
> with INFRA.
> ---
[GitHub] jmeter pull request: Use explicit timeout for TestDNSCacheManager,...
Posted by pmouawad <gi...@git.apache.org>.
Github user pmouawad commented on the pull request:
https://github.com/apache/jmeter/pull/176#issuecomment-204965711
Hi Vladimir,
Exposing public method for testing might introduce issues.
Users can think that field is persisted in JMX while it's not.
Can't it be package protected ?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---