Skip to content
This repository has been archived by the owner on Feb 12, 2022. It is now read-only.

Adds ability to specify a namespace for the Apex classes #14

Closed
wants to merge 1 commit into from
Closed

Adds ability to specify a namespace for the Apex classes #14

wants to merge 1 commit into from

Conversation

tfuda
Copy link
Contributor

@tfuda tfuda commented Dec 11, 2015

Adds ability to specify a namespace for the Apex classes via a new command line argument called test.namespace.prefix. The same namespace will apply to both the test classes and the source classes. Also, updates unit tests to cover this new command line arg.

…es via a new command line argument called test.namespace.prefix. The same namespace will apply to both the test classes and the source classes. Also, updates unit tests to cover this new command line arg.
@vamshi-sfdc
Copy link
Contributor

thanks @tfuda for your contribution / pull - request. In order to move further with this, we need you send the completed "contributor license agreement" available @ https://github.com/forcedotcom/ApexUnit/blob/master/SFDC_CLA.pdf to osscore@salesforce.com

Additional details are available at https://github.com/forcedotcom/ApexUnit/wiki/Contribution-and-Bug-Issue-tracking

Please let us know if you have questions

@tfuda
Copy link
Contributor Author

tfuda commented Dec 18, 2015

The Contributor License Agreement was emailed to osscore@salesforce.com on 12/17/2015.

@vamshi-sfdc
Copy link
Contributor

Thank you very much for sending the CLA. @adarsh-ramakrishna-sfdc Can you please review this pull request?

@vamshi-sfdc
Copy link
Contributor

@tfuda Holiday season Greetings.

@pmedapuram from our team will be helping you further with this.

@pmedapuram
Copy link
Collaborator

Hello @tfuda

We have reviewed the pull request. Your PR has given us the thought to think about this feature. Thank you for that.

We have decided that adding an additional CLI to the existing number of CLIs is not required for this scenario. Your requirement could be met by specifying . in the regex. Here are the use cases and the corresponding functionality.

REGEX FUNCTIONALITY
Classes that satisfy the REGEX in the DEFAULT namespace are tested
*. Classes that satisfy the REGEX in all the namespaces are tested
. Classes in the namespace that satisfy the REGEX are tested

Hope this meets your requirement.

@tfuda
Copy link
Contributor Author

tfuda commented Jan 8, 2016

@pmedapuram Are you saying this is how it works now? Or this is how you think it SHOULD work going forward? Because in looking at the source code in com.sforce.cd.apexUnit.client.utils.QueryConstructor.generateQueryToFetchApexClassesBasedOnRegex, it does not look like this is currently supported.

How would the *.CLASSNAME scenario work if I had two identically named test classes but in different namespaces? Would it run both tests, or just the first one it found? This is actually the problem I was trying to solve with my pull request; I have two classes with the same name, one in the default namespace and one in some other package namespace. Currently, ApexUnit was running the packaged test class when my intent was to run the test class in the default namespace. I would propose that if you use the *.CLASSNAME pattern, it should run ALL classes matching the CLASSNAME pattern, regardless of namepsace.

Aside from that one concern, I agree, this would satisfy our needs, and in fact it is more flexible than the CLI namespace parameter, in that it would allow you to run tests across multiple namespaces in the same run.

I think this behavior could also be extended to the test manifest file as well using, the same syntax (perhaps without wildcard support for the namespace)? And for consistency, it should probably be extended to the source class regex and source class manifest files too, no?

@pmedapuram
Copy link
Collaborator

@tfuda : the functionality mentioned above is currently NOT included in the tool. We have come up with the above functionality from our discussions to address your requirement. We are looking to make the change going forward.

Yes, in the second use case described above,
*. tests all classes matching the regex across all namespaces. In your use case, this would test the classes in the default namespace as well as the class in the package namespace.
. tests all classes matching the regex only in the namespace matching the regex. This is probably what you want.

And yes, this would also enable the user to specify and test classes across multiple namespace.

We are looking to restrict this feature to regex as the manifest files exist for legacy purposes. The intent of the manifest files is for testing legacy classes that were created without the naming convention. Since the requirement can be achieved with regex, we will include the feature here. This will also not entertain users to use manifest files for code that follow a naming convention.

And yes, we could add this feature to regex-for-source-classes. Agreed.

@tfuda
Copy link
Contributor Author

tfuda commented Jan 8, 2016

The enhancements to the behavior of the -regex.for.selecting.test.classes.to.execute option will work fine for selecting the test classes to execute, because we've been pretty good about including the word "Test" in all of our test class names. So, I think we'd be able to use the pattern Test to run all of the tests in our default namespace.

I'm not sure that extending this behavior to the -regex.for.selecting.source.classes.for.code.coverage.computation option for specifying the source classes will work for us. One of our packages has nearly 200 "source" classes and triggers for which we need to compute code coverage. I can't really use a pattern to specify the source classes, since there's no pattern to the class names. Our only reasonable option here is to use the -manifest.files.with.source.class.names.for.code.coverage.computation option with a manifest file that explicitly lists the source class names. As an ISV, we have created a suite of packages that all co-exist within subscriber orgs. In some cases, one package may contain global virtual classes is overridden by classes contained within other packages/namespaces. For example, here's the output of a SOQL query I ran in one of my DE orgs that I'm using to develop one of our manged packages: http://screencast.com/t/dx1ljbz6FG. As you can see, I've got three identically named source classes, differentiated only by their namespace prefix. Without the ability to specify the namespace in the source class manifest file, I'm likely to hit the same class name collision problem when it comes to computing code coverage for the CustomQualification class. My intent is to compute the coverage for the non-namespaced class, but I have no way to guarantee that the ApexUnit framework will pick that class given the current method of de-duping the class names.

@pmedapuram
Copy link
Collaborator

@tfuda : aah I see your use case better now.

We think it is appropriate to add this functionality to manifest files as well. So, both the regex and the manifest files will support namespace prefix. We will let you know once this feature is available in ApexUnit.

@tfuda
Copy link
Contributor Author

tfuda commented Jan 9, 2016

That sounds great. Thanks. I look forward to trying out the enhancement
when it's available.
On Jan 7, 2016 3:27 PM, "pmedapuram" notifications@github.com wrote:

Hello @tfuda https://github.com/tfuda

We have reviewed the pull request. Your PR has given us the thought to
think about this feature. Thank you for that.

We have decided that adding an additional CLI to the existing number of
CLIs is not required for this scenario. Your requirement could be met by
specifying . in the regex. Here are the use cases and the corresponding
functionality.
REGEX FUNCTIONALITY Classes that satisfy the REGEX in the
DEFAULT namespace are tested *. Classes that satisfy the REGEX
in all the namespaces are tested . Classes in the
namespace that satisfy the REGEX are tested

Hope this meets your requirement.


Reply to this email directly or view it on GitHub
#14 (comment).

@pmedapuram
Copy link
Collaborator

@tfuda : In order to unblock you, as of now, we have added a fix to only select classes in the default namespace.
#18

Going forward, we will add the feature to parse . for both manifest files and regex.
(Although the current fix may support specifying namespace in manifest files but not regex)

Regarding calculating code coverage: The tooling API currently does not support querying the ApexClasses in a managed pkg for code coverage metrics.

@tfuda
Copy link
Contributor Author

tfuda commented Jan 14, 2016

Thanks. I'll check this out in the next day or two.
On Jan 13, 2016 7:24 PM, "pmedapuram" notifications@github.com wrote:

@tfuda https://github.com/tfuda : In order to unblock you, as of now,
we have added a fix to only select classes in the default namespace.
#18 #18

Going forward, we will add the feature to parse .
for both manifest files and regex.
(Although the current fix may support specifying namespace in manifest
files but not regex)

Regarding calculating code coverage: The tooling API currently does not
support querying the ApexClasses in a managed pkg for code coverage metrics.


Reply to this email directly or view it on GitHub
#14 (comment).

@tfuda tfuda closed this Jan 16, 2016
@tfuda
Copy link
Contributor Author

tfuda commented Jan 16, 2016

Closing this pull request since an alternate solution is being implemented.

@tfuda
Copy link
Contributor Author

tfuda commented Jan 20, 2016

@pmedapuram. I tried this out. I have some comments/concerns about the way
the -regex.for.selecting.source.classes.for.code.coverage.computation
option works. Right now, since I have so many classes in my org, I am using
a "regex" of * to select ALL classes in my default namespace for code
coverage computation. This is causing the code coverage calculator to
include all of the Test classes that I've already selected via
the -regex.for.selecting.test.classes.to.execute option. This results in
artificially low overall code coverage numbers due to the inclusion of test
classes in the code coverage computations. I feel that to be useful, the
regex for selecting source code classes should really be excluding classes
that are annotated with the @istest annotation. Unfortunately, there's no
way to query for this via SOQL against the ApexClass table. But the good
news is that this information can be obtained via the Tooling API (which
you are already accessing to get the code coverage numbers). The following
Tooling query will return information about the ApexClasses in the system:

/services/data/v35.0/tooling/query?q=select+SymbolTable+FROM+ApexClass

Here's an example of one record returned by this query as viewed in
Workbench: http://screencast.com/t/QFUnnSg5. You can see that it is
possible to identify classes the have the IsTest annotation in this manner.
Just something to think about as a possible future enhancement to make
these "regex" options for selecting test classes and source classes more
useful.

BTW, calling these regexes is not really accurate, since what you're really
doing is just substituting the * characters for % characters and feeding
the results into a SOQL LIKE clause. True regex pattern matching is much
more powerful, but it would require querying for ApexClasses and then post
processing them using the Java Pattern and Matcher classes to support true
regex matching.

On Wed, Jan 13, 2016 at 7:30 PM, Tom Fuda tom@9summer.com wrote:

Thanks. I'll check this out in the next day or two.
On Jan 13, 2016 7:24 PM, "pmedapuram" notifications@github.com wrote:

@tfuda https://github.com/tfuda : In order to unblock you, as of now,
we have added a fix to only select classes in the default namespace.
#18 #18

Going forward, we will add the feature to parse .
for both manifest files and regex.
(Although the current fix may support specifying namespace in manifest
files but not regex)

Regarding calculating code coverage: The tooling API currently does not
support querying the ApexClasses in a managed pkg for code coverage metrics.


Reply to this email directly or view it on GitHub
#14 (comment).

Tom Fuda
Nine Summer
Email: tom@9summer.com
Phone: +1.203.361.9416
http://9summer.com : Accelerating innovative products to market.

@pmedapuram
Copy link
Collaborator

Hey @tfuda

This is a valuable feedback. Yes, I agree that our regex option does not really respect the "REGEX" syntax. Using Pattern, we could enhance the regex parsing and your use case could be used as a data point to build in an additional feature to remove the isTest annotated classes.

I see this as different from the current namespace issue. Could you create a new issue and add the same content. We will discuss and get back to you.

@tfuda
Copy link
Contributor Author

tfuda commented Jan 20, 2016

@pmedapuram - Done... see #21

@tfuda
Copy link
Contributor Author

tfuda commented Jan 20, 2016

I also opened a separate issue for the exclusion of test classes. See #22.

On Wed, Jan 20, 2016 at 1:54 PM, Pavan Tej Medapuram <
notifications@github.com> wrote:

Hey @tfuda https://github.com/tfuda

This is a valuable feedback. Yes, I agree that our regex option does not
really respect the "REGEX" syntax. Using Pattern, we could enhance the
regex parsing and your use case could be used as a data point to build in
an additional feature to remove the isTest annotated classes.

I see this as different from the current namespace issue. Could you create
a new issue and add the same content. We will discuss and get back to you.


Reply to this email directly or view it on GitHub
#14 (comment).

Tom Fuda
Nine Summer
Email: tom@9summer.com
Phone: +1.203.361.9416
http://9summer.com : Accelerating innovative products to market.

@pmedapuram
Copy link
Collaborator

Cool, thank you @tfuda

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants