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

Make List of Apex Class Names from Delta Package.xml to Use In Delta Deployment with Test Run #126

Closed
nickytorstensson opened this issue Apr 12, 2021 · 15 comments · Fixed by #139
Assignees
Labels
enhancement New feature or request test classes Anything related to apex test class

Comments

@nickytorstensson
Copy link

nickytorstensson commented Apr 12, 2021

Is your proposal related to a problem?


It is fantastic, that the SGD generates a delta package.xml, which is used to deploy the delta. However, if the delta package.xml file contains apex classes (and apex test classes), there is no easy way to generate a list of apex classes to use for the RunSpecifiedTest part of a validation run

If the SGD plugin also could generate such a list, it would make it so easy to also run selected tests in a validation run, further enhancing the delta deployment experience.

E.g. the command for running specified tests:
sfdx force:source:deploy --testlevel RunSpecifiedTests --runtests 'MyApexClass1, MyApexClass1_Test, ... '

Describe the solution you'd like


If the SGD command generates the following package.xml file:

<?xml version="1.0" encoding="UTF-8"?>
<Package xmlns="http://soap.sforce.com/2006/04/metadata">
    <types>
        <members>MyApexClass1</members>
        <members>MyApexClass1_Test</members>
        <members>...</members>
        <name>ApexClass</name>
    </types>
    <version>51.0</version>
</Package>

Then I want the SGD plugin to also generate a .txt file (e.g. ApexClassNames.txt) or return a string. The file/string should contain the names of apex classes in above package.xml file (both normal classes and test classes), separated by commas.
E.g. content is string with value MyApexClass1, MyApexClass1_Test, ...

If .txt file, it can be placed in a new folder, name is optional, to be decided.

Describe alternatives you've considered


The alternative is to generate a script, which gets the package.xml file from the SGD method, access the type with name ApexClass and retrieve all members from the section. Not the prettiest solution, and script will vary between each CI/CD developer, who has this need.

@nickytorstensson nickytorstensson added the enhancement New feature or request label Apr 12, 2021
@nickytorstensson nickytorstensson changed the title Make File with Apex Class Names in Delta Package.xml to Use In Delta Deployment with Test Run Make List of Apex Class Names in Delta Package.xml to Use In Delta Deployment with Test Run Apr 12, 2021
@nickytorstensson nickytorstensson changed the title Make List of Apex Class Names in Delta Package.xml to Use In Delta Deployment with Test Run Make List of Apex Class Names from Delta Package.xml to Use In Delta Deployment with Test Run Apr 12, 2021
@scolladon
Copy link
Owner

Hi @nickytorstensson !

Thanks for this great suggestion.

I open the solution debate. Should we

  • Add this output in the current sgd:delta:generate command ? Then should it be triggered via a specific parameter or in all case ? How to drive the output ? Both in a file and in the json output ?
  • Create another command to do it from a package.xml list. So it can be reused in other context
  • Document a xpath transformation using yq

@nickytorstensson
Copy link
Author

Hi @scolladon

  1. I think, since the enhancement is relevant for every execution of the sfdx sgd:source:delta command, it should be part of that current command. I don't foresee a need for additional parameters. I think it would be best to have the file generated, containing the list. Additionally returning the output in JSON could be relevant too, but would not be relevant in my scenario (could be a future change request).
  2. I understand it would be good with a separate function for single use, but I would assume that implies that the package.xml is modified after the sfdx sgd:source:delta command, which I don't expect (but of course could be relevant in some edge cases).
  3. I am not familiar nor experienced with yq / jq, so I will rely on your experience on that.

I hope my answers help :-)

@mehdicherf
Copy link
Collaborator

mehdicherf commented Apr 13, 2021

Deciding exactly which tests to run with --testlevel RunSpecifiedTests can be a tricky business: some teams will run only the new and modified classes, other teams will rely on their test naming convention to list the tests classes to execute, some other teams will have a baseline of test classes that are run under certain circonstances ("if something is changed on Account, always run AccountTriggerHandler_Test"...)
So I feel that if we start offering an output of the test classes to run in SGD, there will be an expectation that we support those (or some of those) different strategies. It would be great in the long-term, but would definitely require more work.

So, If we "only" need the list of Apex classes from the package.xml, using a tool such as yq, as @scolladon suggested, seems to me like that the best short-term solution. Here is an example of a command to output the list of Apex classes:
cat package/package.xml | xq . | jq '.Package.types | select(.name=="ApexClass") | .members | join(",")'

EDIT: a more robust version would be xq . < package/package.xml | jq '.Package.types | [.] | flatten | map(select(.name=="ApexClass")) | .[] | .members | [.] | flatten | map(select(. | index("*") | not)) | unique | join(",")' -r

@nickytorstensson Would that solution meet your need?
(note that you can also add additional transformation before the | join(","), if needed)

@kknapp1
Copy link

kknapp1 commented Apr 13, 2021

I agree with @mehdisfdc 's comments and offer my own solution as an example for discussion.

@nickytorstensson, I am using SGD for the same use case as you've described. I use the --generate-delta option to create a folder of changes in the build directory, then use a simple PowerShell script to find the test classes. Something similar could be done with Python or your scripting language of choice.

#(very basic PowerShell example)
$files = Select-String -Path "*Test.cls" -Pattern "isTest","testMethod" -List | Get-Item
foreach ($f in $files) {
    $testList += $f.BaseName + ","    
}

I use this $testList string as the input to the RunSpecifiedTests option.

As our testing pipeline matures, we will be selecting tests much like @mehdisfdc suggests: some baseline suites, added/updated tests, or possibly even detecting dependencies in code and running related tests so the usefulness of SGD outputting test class names will diminish.

@nickytorstensson
Copy link
Author

Hi all,

First, thanks for sharing your input and considerations!

I agree, that every team has their own conventions for selecting, which test classes need to run. And that definitely makes it difficult to develop something general, if everyone will not be using it the same way. However, my suggestion does not focus on the individual team's needs, but the general need, which I believe should apply to all teams.

  • As an argument for getting all added or modified apex classes:
    In the context of an apex class; if you as a developer modify it, then (in principle) you should also update your test class to accommodate for the changed logic. As such, I think it is still valid to include all added or modified apex classes for a RunSpecifiedTest run, and this would be a general use case for any team (in my opinion).

  • Countering my own argument:
    By example; though fixing typos and simple refactoring will likely not change the test class, it would in principle be redundant to modify the test class by just adding a line break or space for the sole purpose of getting it included.
    I know, the workaround in this example introduces an extra step, but for a small cost.

@mehdisfdc, considering that introducing such functionality will likely result in expectations for extended functionality, then I also agree with scolladon, that this should be in a separate command, such that it can scale well.

On your examples for getting the apex classes into a variable, I would have to look into that (still a novice user ;-), so would have to install the programs on my Docker image and run some tests)

Best, Nicky

@scolladon
Copy link
Owner

Hi all,

I personally prefer the solution using yq.
Because

  • it is very simple
  • it relies on a very trusted tool
  • it is simple to use in local and in CI/CD as well
  • it fulfils the requirement (do the job)
  • it is flexible enough to be adapted to fit the most demanding use cases
  • it allows the plugin to not reinvent the wheel and to stay focus on what it does best.

I propose to document the usage of yq in the README.md using the query designed by @mehdisfdc to show case how to run Specified Test using this method.
It could be a good (crawl) answer to the requirement.

@RohitGagan16
Copy link

@scolladon, i dont think the solution works!! for fetching the test classes that covers the delta, arent we suppose to fetch the dependencies? and also it should cover the apex triggers as well.

Please let me know the take on this if someone implemented the solution to run the test classes from the delta package
@nickytorstensson

@nickytorstensson
Copy link
Author

Hi @RohitGagan16,
Unfortunately, I did not get to implement it, nor see it implemented.

I agree, I still think it is a relevant feature to have, and instead of each user of the plugin doing as scolladon suggests in his latest comment, it would be preferable that it is a feature of the plugin itself.

In addition, I see mehdisfdc referred to this issue in 402

@mehdicherf
Copy link
Collaborator

@RohitGagan16 @nickytorstensson are you referring to compiling a list of the Apex classes in the delta, so that it can be used in a RunSpecifiedTests deployment?

If this is what your referring to, you'll find an example GitHub actions pipeline that does it here:

export APEX_CLASSES=$(xq . < package/package.xml | jq '.Package.types | [.] | flatten | map(select(.name=="ApexClass")) | .[] | .members | [.] | flatten | map(select(. | index("*") | not)) | unique | join(",")' -r)

@Ronak-Toshniwal
Copy link

Ronak-Toshniwal commented Jan 27, 2023

I took mehdisfdc's string and made few changes to meet my requirement so just shareing if anyone also want's to use it.
Updated script works on common naming convention. It will auto add testfile name in runspecified test even if test class in not available in package.xml
Working:
it will look for word "TEST" (Case Sensitive) after each class name if class has test then it will ignore and add it to the string to put in runspecifiedtests string, If NOT it will put TEST at the end of class name and will be concatenated at the end of classes string.

.Package.types | [.] | flatten | map(select(.name=="ApexClass")) | .[] | .members | [.] | flatten | map(select(. | index("*") | not)) | unique | map(select(. | endswith("TEST") != false | not )) | join("TEST,")

Example
https://jqplay.org/s/b08WzWI2JQV

Above string puts all and only the classes and test classes which are mentioned in package.xml. Where as if your org followed naming convention then you can use the above and it will reduce to put mock commits to include it in runspecified test.

NOTE: It will not work in case you are using --generate-delta because in that case it will look for files in that folder and only files available in package.xml are available in that folder.

@scolladon
Copy link
Owner

scolladon commented Jan 27, 2023

Great sharing @Ronak-Toshniwal
Thanks for the hard work here.

Why do you think it will not work in case we use -d ? To me, if it takes it from the package.xml or the folder it is equivalent because what is in the package.xml is in the delta folder.

Based on your work here, the README and this discussion I have been able to come with those jq queries:

  • Return apex test class related to non test class in the package.xml: Append a postfix (TEST) to every class (excluding class with the postfix already) and return only those classes (snippet)
    .Package.types | [.] | flatten | map(select(.name=="ApexClass")) | .[] | .members | [.] | flatten | map(select(. | index("*") | not)) | map(select(. | index("TEST") | not)) | map(. + "TEST") | unique | join(",")
  • Return apex text class in the package.xml: Extract every test class with postfix (TEST) (snippet)
    .Package.types | [.] | flatten | map(select(.name=="ApexClass")) | .[] | .members | [.] | flatten | map(select(. | index("*") | not)) | map(select(. | index("TEST"))) | unique | join(",")
  • Return apex test class related to non test class in the package.xml and the test class in the package.xml: Append postfix (TEST) to every class and return class with the postfix already (snippet)
    .Package.types | [.] | flatten | map(select(.name=="ApexClass")) | .[] | .members | [.] | flatten | map(select(. | index("*") | not)) | map(. | sub("TEST$" ; "") + "TEST") | unique | join(",")

Let me know if it helpful, we could create a PR to enrich the README

@RohitGagan16
Copy link

Hi, Just fetching the test classes from the delta is not enough. Rather every dependency of the delta componnet needs to fetched and make sure you have dependent test classes for this. That will solve the problem. So the solution specified above by @mehdisfdc / @Ronak-Toshniwal is not enough

@scolladon
Copy link
Owner

Hi @RohitGagan16 !

I remember us discussing this approach here!

I'm not sure it is the right way to solve that problem, following software engineering best practices with proper unit testings would avoid us to have to calculate this dependency graph. We would be able to rely on those simple jq query then.

In order to know what test classes to call to get the minimum coverage we need to deploy just this class would require to have all the coverage, so it would require to run all test with coverage prior to that. And it will require some graph search algorithm implementation (like a shortest path to get 75% coverage for the classes in the package.xml). It is a lot of work where the solution should not be approached by tooling, it should be approached by clean software engineering practices.

Maybe we should open a discussion to sum this up and follow our thought there
What do you think guys ? Should I do it ?

@Ronak-Toshniwal
Copy link

Hi @scolladon,
according to me it will not work in case of --generate-delta because for that we need to have test class also modified or added in PR/Commits then only it will track and add into package.xml but what if i modified abc.cls (class file) but i want at the time of deployment if it's test class is not available in package.xml it will through error because of coverage error.

so i wanted to add testclass automatically and if we use delta then that file will not be available in delta package and it will throw no source backed component error.
In that case we need to have access to ORIGINAL force-app folder so all test classes are also accessible.

hope you got my point.

@scolladon
Copy link
Owner

I'm not sure to follow here @Ronak-Toshniwal

Let's say we have a force-app folder with a class and its test class (both already deployed and known from the system):

force-app
|__ classes
	  |__ MyClass.cls
	  |__ MyClassTest.cls

We make a change to MyClass.cls and then generate incremental package:

$ sfdx sgd:source:delta -f "HEAD~1"  -d

It generates this folder structure:

output
|__ destructiveChanges
	|__ destructiveChanges.xml
	|__ package.xml
|__ force-app
	|__ classes
		  |__ MyClass.cls
|__ package
	|__ package.xml

And this package.xml:

<!-- cat output/package/package.xml -->
<?xml version="1.0" encoding="UTF-8"?>
<Package xmlns="http://soap.sforce.com/2006/04/metadata">
    <types>
        <members>MyClass</members>
        <name>ApexClass</name>
    </types>
    <version>57.0</version>
</Package>

Then we can deploy either using package.xml or delta source folder:

$ sfdx force:source:deploy -x output/package/package.xml
$ sfdx force:source:deploy -p output/force-app

What I understand, according to you, it is not be possible to list test classes to run in specified test if they are not present in the sources deployed. It throws no source backed component error.
Whereas it would be possible to specify test class to run not present in the package.xml if the classes were deployed using only the package.xml.

So, as the MyClassTest is not part of the deployment (but already in the sandbox) we should have the following result:

$ sfdx force:source:deploy -x output/package/package.xml -l RunSpecifiedTests -r "MyClassTest" # success
$ sfdx force:source:deploy -p output/force-app -l RunSpecifiedTests -r "MyClassTest" # failure

So your point is: when using -d, sgd should detect test classes (via a naming convention or other) and add them in the sources deployed / package.xml, even if they are not listed by the git diff, right ?
I'm not convinced by this approach, to me, it is possible to specify test classes to run while deploying, even if those test classes are not part of the package.xml or the source folder deployed. The classes just need to be in the target environment to be run by the specified tests.

I played a bit with this and was able to deploy using both package.xml or source folder and specifying test not present in them but in the platform.
Using package.xml:
Screenshot 2023-01-29 at 12 54 19

Using source folder:
Screenshot 2023-01-29 at 12 54 29

I created a branch in our playground if you want to play with as well. Everything done here is in that branch

$ git clone https://github.com/scolladon/sfdx-git-delta-reproduction-playground.git
$ cd sfdx-git-delta-reproduction-playground
$ git checkout issue/126
$ sfdx force:source:deploy -p force-app # init

To generate the delta folder execute this command:

sfdx sgd:source:delta -f "HEAD~1" -d   

Then deploy MyClass alone while specifying MyClassTest to run !

$ sfdx force:source:deploy -p output/force-app -l RunSpecifiedTests -r "MyClassTest" # with source
$ sfdx force:source:deploy -x output/package/package.xml -l RunSpecifiedTests -r "MyClassTest" # with package.xml

I looked around for the no source backed component error and found this old issue. I was not able to understand the link with our use case here.
Can you help me reproduce it locally so I can better understand how this error work please ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request test classes Anything related to apex test class
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants