-
Notifications
You must be signed in to change notification settings - Fork 309
Run Unit Tests from VBA or other COM-Aware programs #6270
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
Conversation
Run tests async and parse programmatically - Modified `RunWithResults` in `TestEngine.cs` to include a parsing mechanism that waits for completion before running tests. - Changed the implementation of `RunAllTestsAndGetResults` to run tests asynchronously and log results to a specified file.
❌ Build Rubberduck 2.5.92.6374 failed (commit fd403627c9 by @DecimalTurn) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like a good start! I think some wrinkles need a bit of ironing out before it's merged, but this is very promising, good job!
Let's find a way to invert the provider/extension dependency so that the extension class remains unreferenced, as an entry point class should be (or perhaps I missed something?)
Similarly, the TypeLibAPI/provider dependency needs to be inverted so that the lower-level API gets injected into higher-level services, not the other way around.
I think it's worth taking the time to serialize everything we know about the test results (name, outcome, duration, categories, etc.) into a JSON string - the engine itself would return the results (objects, not strings) and the exposed service would be responsible for outputting the serialized JSON string. It makes a bit more work on the .yml side to format things, but then it's also more flexible and a script could decide to filter categories and/or outcomes instead of outputting everything.
Overall it's not too far off at all, just a bit of polish and it'll be a very nice addition to Rubberduck's feature set!
// Get the TestName, but stop at the first \r\n to get only the signature | ||
int index = result.TestName.IndexOf("\r\n"); | ||
var signature = index >= 0 ? result.TestName.Substring(0, index) : result.TestName; | ||
resultBuilder.AppendLine($"{result.Result.Outcome}: {signature}"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure why the signature is relevant, seems to me just the name is sufficient (besides they're all going to be Public Sub
procedures).
Thinking the results should probably be output as a json string, and let the client display it the way they want it, no?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, the name would be sufficient. Is there a way to get the name of the procedure from a TestResult
? I had to create the struct TestInfo
as a way to get something close to the method name, but then we need to parse it to get only the name.
|
||
public VBETypeLibsAPI_Object(IVBE ide) | ||
public VBETypeLibsAPI_Object(IVBE ide, object testEngineProvider) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure about this one, seems to be mixing up abstraction levels a bit.
{ | ||
public class TestEngineProvider | ||
{ | ||
private _Extension _extension; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There has to be another way to do this... having the entry point class as a dependency feels very backwards here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is backwards indeed and I was actually hoping you'd have suggestions on how to do this better. The problem is that SetAddInObject
is called pretty early in the initialization process, so there isn't much to grab onto. Could it be moved further down the line without breaking things?
✅ Build Rubberduck 2.5.92.6375 completed (commit f67fcc6866 by @DecimalTurn) |
The .yml file can be removed, ...but it would be interesting to include a sample GitHub Action that installs RD and runs the tests of a specified VBA project, like you showed on Discord! |
Rubberduck.Main/Extension.cs
Outdated
// FOR DEBUGGING/DEVELOPMENT PURPOSES, ALLOW ACCESS TO SOME VBETypeLibsAPI FEATURES FROM VBA | ||
_addin.Object = new VBETypeLibsAPI_Object(_vbe); | ||
// FOR DEBUGGING/DEVELOPMENT/CLI PURPOSES, ALLOW ACCESS TO SOME VBETypeLibsAPI FEATURES FROM VBA | ||
_addin.Object = new VBETypeLibsAPI_Object(_vbe, new TestEngineProvider(this)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think part of the solution will entail not exposing the TypeLibAPI directly, but rather a service that can receive both the API and the test engine /provider, and then maybe exposing the API object as a property of that class, while also exposing the test engine as another property, and that one can be fully visible and documented if it needs to be. That'll require registering a couple more GUIDs for these new COM types; they're in static classes under Rubberduck.Resources. Basically the premise was that we wanted to maybe expose the TypeLibAPI there, but instead we'll have a dedicated API that still exposes the hidden TypeLibAPI but provides a visible CLI unit testing functionality.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That makes a lot of sense! So the service could be named ExternalAPI
and it would contain VBETypeLibsAPI
(we might want to drop the "_Object" in the name) and TestEngineAPI
.
TestEngineAPI
could have an Initialize
method that we call right after _container
is set with TestEngineAPI.Initialize(_constainer.Resolve<ITestEngine>());
.
Regarding the GUIDs and COM types registration, that's new to me, so I'll need to have a look into it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IIRC the _Object one is a COM wrapper around the real internal API so let's just leave it alone, but looking at the attributes in its definition will show you what I mean about the GUIDs.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just to be clear, is this something like this that you had in mind?
Also, I'm getting the following error as soon as I try to add IVBETypeLibsAPI_Object as a member of the interface:
The command "call "C:\Program Files\Microsoft Visual Studio\2022\Community\Common7\Tools\sDevCmd.bat"
midl.exe /win32 /tlb "Rubberduck.x32.tlb" "Rubberduck.idl" /out "C:\Users\DecimalTurn\AppData\Local\Temp\RubberduckMidl"
midl.exe /amd64 /tlb "Rubberduck.x64.tlb" "Rubberduck.idl" /out "C:\Users\DecimalTurn\AppData\Local\Temp\RubberduckMidl""
exited with code 2026.
namespace Rubberduck.ExternalApi
{
[
ComVisible(true),
Guid(RubberduckGuid.IExternalAPIInterfaceGuid),
InterfaceType(ComInterfaceType.InterfaceIsDual),
EditorBrowsable(EditorBrowsableState.Always)
]
public interface IExternalAPI
{
[DispId(1)]
void InitializeAPIs(ITestEngine testEngine);
[DispId(2)]
ITestEngineAPI TestEngineAPI { get; }
[DispId(3)]
IVBETypeLibsAPI_Object VBETypeLibsAPI { get; }
}
[
ComVisible(true),
Guid(RubberduckGuid.ExternalAPIObjectGuid),
ProgId(RubberduckProgId.ExternalAPIObject),
ClassInterface(ClassInterfaceType.None),
ComDefaultInterface(typeof(IExternalAPI)),
EditorBrowsable(EditorBrowsableState.Always)
]
public class ExternalAPI : IExternalAPI
{
private readonly IVBETypeLibsAPI_Object _vbeTypeLibsAPI_Object;
private ITestEngineAPI _testEngineAPI;
public ExternalAPI(IVBETypeLibsAPI_Object vbeTypeLibsAPI_Object)
{
_vbeTypeLibsAPI_Object = vbeTypeLibsAPI_Object;
}
public void InitializeAPIs(ITestEngine testEngine)
{
_testEngineAPI = new TestEngineAPI(testEngine);
}
public IVBETypeLibsAPI_Object VBETypeLibsAPI { get => _vbeTypeLibsAPI_Object; }
public ITestEngineAPI TestEngineAPI
{
get
{
if (_testEngineAPI == null)
{
throw new InvalidOperationException("TestEngineAPI is not initialized.");
}
return _testEngineAPI;
}
}
}
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Something like it, yes. Probably needs some deeper tweaks with generating the .tlb for it to work, but that can happen later.
✅ Build Rubberduck 2.5.92.6376 completed (commit f837087b81 by @DecimalTurn) |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## next #6270 +/- ##
=======================================
Coverage 97.58% 97.58%
=======================================
Files 4 4
Lines 413 413
Branches 28 28
=======================================
Hits 403 403
Misses 6 6
Partials 4 4 🚀 New features to boost your workflow:
|
That would be desirable indeed to have more information. I think that if the aim is to make this readily available for CLI purposes, the CSV format might be friendlier to parse and display in the console without requiring external libraries.
I can certainly do that. The .yml would be much simpler if we wait for the next version of VBA-Build since it will include options for testing. However, if the goal is to have a minimum reproducible example, I could try to see if I can strip down the Powershell scripts to make it possible to inlcude directly in a sample .yml file. |
.github/workflows/build.yml
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi. This is really cool and I think something in everyone's mind, that RD testing (and eventually RD inspections and auto format) should be ci/cd or pre-commit runnable.
Currently if I understand correctly, the inclusion of this file will expose a rubberduck build action, will that be available to any repo in this current format?
How would you feel about including in this pr the run-tests action? If it can be a standard format like json, other repos could chop and change or analyse the results. But it feels like exposing the base action from the RD repo itself would provide a standard way to do things, and make it clear how the com API must be supported (so changes to RD consider the gh actions and don't break them).
Just throwing the idea out there. Feels like adding the com exposure but not adding the gh action it enables is a miss.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi, thanks for the feedback! It would indeed be something that needs to be at least documented somewhere to make it available for everyone.
Currently if I understand correctly, the inclusion of this file will expose a rubberduck build action, will that be available to any repo in this current format?
Yes, that would expose a build action to build the Rubberduck's DLLs, but in its current form, the build action won't create the .exe
installer. While testing the new feature, I used that .yml
file to create a pre-release build on my feature branch, but I still used the latest official installer to make the install. It's only after the install was done that I replaced the DLLs from my feature branch's build.
How would you feel about including in this pr the run-tests action? If it can be a standard format like json, other repos could chop and change or analyse the results. But it feels like exposing the base action from the RD repo itself would provide a standard way to do things, and make it clear how the com API must be supported (so changes to RD consider the gh actions and don't break them).
As I mentioned in this comment, I don't mind adding a run-tests action. The simplest way would be to use something like this:
name: Build and Test VBA
on:
- workflow_dispatch
jobs:
build-and-test:
runs-on: windows-latest
steps:
- name: "Checkout"
uses: actions/checkout@v4
- name: "Build VBA-Enabled Documents and Perform Tests"
uses: DecimalTurn/VBA-Build@dev
with:
sourceDir: "./src"
timeout-minutes: 20
- name: "Upload Build Artifact"
uses: actions/upload-artifact@v4
with:
name: "VBA-Enabled-Documents"
path: "./src/out/*"
if-no-files-found: warn
The reason why I'm using @dev
in this example is just that the action that runs the test is only available in the dev branch at the moment, but it will be available in the next release.
I'm leaning towards using my VBA-Build action because the setup needed to make testing work is not trivial and having all that PowerShell code crammed into a .yml
file doesn't seem like the cleanest approach.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just had a quick look at the PowerShell scripts involved; I think the API could just as well expose tooling for importing source files from a folder - the functionality for it is already there in RD, just needs to be exposed and then the script could just do something like $rubberduck.ImportSourceFiles($path)
before running the tests.
And... yeah this does open up the possibility of also exposing inspection results, since we're apparently able to trigger a parse from that environment.
"Official" support is uncertain though: it remains a case of running Office automation on a server environment, which isn't a use case Microsoft supports; Office instances may eventually no longer be included in windows-latest (?) images, who knows.
What happens if, despite all warnings, a user pops a MsgBox in a unit test?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you forcibly moq some common interaction elements so they instead raise an error or assert.fail?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's probably a way to set up the Fakes API to intercept everything and throw on invoke (would probably induce some noticeable overhead), but then it needs a way to somehow prioritize whatever setup a test that uses the Fakes API might want or need... not sure how to go about this.
Let's think about it.
Test run starts; before each test, the engine optionally (perhaps unless specified otherwise) receives a "this is a headless run" flag that makes it set up all fakes to throw some MissingFakesSetupException. Not sure we want to get picky about which specific hooks should be throwing: every single one of them has very good reasons to exist and failing to set them up when a test calls any of these functions will definitely screw up a test. Still I don't think we want this behavior in non-headless scenarios, because of the overhead.
The first test sets up Fakes.MsgBox to test some user-driven yes/no branching logic; test engine unhooks the throwing Fakes.MsgBox, hooks the user-specified behavior in its place.
Test invokes the branching logic which calls the MsgBox function; the call is intercepted and the test keeps running. Branching logic then invokes the InputBox function which wasn't configured by the test; the call is intercepted and the test fails with a descriptive MissingFakesSetupException. Fakes get unhooked.
Test run proceeds with the next test, until all tests have been executed.
Test run concludes and outputs the serialized results.
It might work.
I remain convinced that the output should ultimately be de/serialized with System.Text.Json; no additional libraries are needed, and the deserialized results can be easily consumed as objects; the rubberduck-vba org could own a "FormatTestResults" GH Action that accepts the JSON string and could filter and format it accordingly with some parameterization, that way any change in the test engine's output could be reflected in the GH Action that accepts and formats it, without forcing every user to update their CI scripts.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just had a quick look at the PowerShell scripts involved; I think the API could just as well expose tooling for importing source files from a folder - the functionality for it is already there in RD, just needs to be exposed and then the script could just do something like $rubberduck.ImportSourceFiles($path) before running the tests.
That's true, Rubberduck could take care of importing the VBA code, but I don't think there is anything in RD to recreate an office document from XML source. I guess you could always just include a template file pre-assembled instead.
However, you still need to install Office via Chocolatey (it's not part of the windows-latest). Then you have to make some registry edits to enable access to the VB object model. Otherwise, the Rubberduck-API won't be accessible.
"Official" support is uncertain, though: it remains a case of running Office automation on a server environment, which isn't a use case Microsoft supports; Office instances may eventually no longer be included in windows-latest (?) images, who knows.
What happens if, despite all warnings, a user pops a MsgBox in a unit test?
Yeah, I wasn't expecting "official" support for this since it's still experimental. I'm however motivated to find ways to make this work and see if there are workarounds when needed. Regarding the Office installation, there should be other ways as well than to use Chocolatey: downloading the Office installer directly from the web and is still an option.
The weird thing is that Office doesn't even need to be activated to run VBA or interact via COM automation as shown in the screenshot below:
And yes, that screenshot was taken on the windows runner's virtual monitor using a PowerShell script. For now, I've also programmed a screenshot to be taken in my integration tests when the process fails or hangs, so that way you see how things looked like at the end.
For the case of a MsgBox, I have another PowerShell script in the works to deal with that, so we'll see.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would you be open to move VBA-Build under the rubberduck-vba org [...]] ?
I could be convinced, yes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perfect, so there's no chance this just randomly stops working if it's the script that's installing Office.
Not a chance RD is going to build a host document from XML, but with RD loaded and running I'm pretty sure we can tell the host to create a new document (if it doesn't open with an empty document in the first place) and expose a method that takes a path and have Rubberduck do the importing of the source files using the very same command you'd be invoking from the Code Explorer context menu; I'll look into this.
As for the GH Action, I'd like to treat it as a part of the "run VBA unit tests with GitHub Actions" package, meaning the actions themselves would basically be a Rubberduck feature to make the functionality reasonably simple to use - from what I can tell each action needs to be its own repository, so they'd probably be something like these:
-
rubberduck-vba/action-build
Sets up a Windows environment, installs and registers everything needed, imports the source files into an empty project, parses the project and succeeds if the parser state ends up ready, fails with exception details if there's a problem at any point. -
rubberduck-vba/action-test
Accesses the Rubberduck add-in in the already configured Windows environment (runs the build action if missing?), initiates a headless-mode test run, and if successful spits out a result object that contains all the test details, outcomes, log messages, etc., otherwise fails with exception details. -
rubberduck-vba/action-format-tests
Accepts the headless test result object and filters and summarizes and formats the data, and outputs it to the console as configured. Fails with invalid input.
And eventually:
-
rubberduck-vba/action-inspect
Accesses the Rubberduck add-in in the already configured Windows environment (runs the build action if missing?), optionally configures inspection severity levels (an XML configuration file/path could be supplied for this) and triggers a parse run (or not, if there's no config override), if successful spits out a result object that contains all the inspection results (also code metrics?), otherwise fails with exception details. -
rubberduck-vba/action-format-inspections
Accepts the headless inspect result object and filters and summarizes and formats the data, and outputs it to the console as configured. Fails with invalid input.
Having these under the rubberduck-vba org more strongly associates them with Rubberduck, and makes a cohesive set of features that definitely look and feel like they're a part of a whole.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note for a RD3 they would be quite different so they make sense in this repo specifically I would say
Also makes them discoverable as GitHub gives you a button on the repo banner if it has actions you can use
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not a chance RD is going to build a host document from XML, but with RD loaded and running I'm pretty sure we can tell the host to create a new document
Then maybe VBA-Build could remain independant and I can help you set up the different actions for RD.
Having these under the rubberduck-vba org more strongly associates them with Rubberduck, and makes a cohesive set of features that definitely look and feel like they're a part of a whole.
You could then even have a global action that combines them all with inputs allowing GitHub users to customize the behavior.
✅ Build Rubberduck 2.5.92.6377 completed (commit 7d61534c63 by @DecimalTurn) |
❌ Build Rubberduck 2.5.92.6378 failed (commit c03979500f by @DecimalTurn) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've pulled the branch and will be making a couple of small changes around the deferred initialization, but I'll merge this PR as-is for now.
...right after AppVeyor CI passes (looking into it) |
Closing for now, it won't work as-is - I have most of these commits in another branch now and there are still some issues left to address before I can make a new PR, but I want to see this feature delivered, your work here isn't lost. |
No, worries. I'm glad I managed to get the ball rolling. I'll let you take it from here. Regarding the implementation for the gh actions, we can discuss them afterwards. |
The motivation of this PR is to allow VBA or other COM-Aware programs to run tests by calling the Rubberduck Addin Object directly. See #6269
In order to do that, the first thing that needs to be done is to make
SetAddInObject
compile in the Release build (not only in Debug). I don't think this is problematic, since you really have to look for it to useVBE.AddIns("Rubberduck.Extension").Object
. Because the methods are not even visible with Intellisense, I assume it won't affect the normal end-users unless they know about the feature.One of the challenges of providing access to the TestEngine was that when it's time to set the _addin.Object and create a new
VBETypeLibsAPI_Object
,_container
isn't ready to supply it, so we need to pass aTestEngineProvider
that will retrieve the TestEngine later when the addin is fully initialized.The rest of the PR is simply the introduction of
RunWithResults
in the TestEngine and its callerRunAllTestsAndGetResults
insideVBETypeLibsAPI
.I remain open to comments and suggestions if there is a simpler way to achieve this.
Additional notes:
StringLineBuilder
within the TestEngine, I had to move it to the InternalApi.Common namespace.