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

Unit / Integration tests #86

Open
adiessl opened this issue May 10, 2022 · 2 comments
Open

Unit / Integration tests #86

adiessl opened this issue May 10, 2022 · 2 comments

Comments

@adiessl
Copy link
Contributor

adiessl commented May 10, 2022

First of all, thank you for providing this NuGet package @koenvzeijl and @sleeuwen (and thanks for the swift merges of my PRs 😉). We have been able to integrate it into our development process where it replaced LibSassBuilder. Its advantage is not only that it is using Dart Sass, it also seems to do the compilation quite a bit faster. Comparing the compilation times on our build machines reveals that AspNetCore.SassCompiler only uses around 20 percent of the time that LibSassBuilder previously used.

While digging through the source code I noticed that there are currently no unit or integration tests and wondered if you thought about adding some, for example to make sure that future Dart Sass versions do still adhere to the same console output that the regular expression expects. Maybe one of you is already working on it? If not, I might be able to contribute some tests.

@sleeuwen
Copy link
Collaborator

Hi @adiessl , thank you for all the PRs you created and help make the library better. It's awesome to hear you were able to improve the compilation time so much compared to LibSassBuilder .

After the last few issues we had with the new releases I also talked with @koenvzeijl that we will need to improve our tests situation to hopefully catch these bugs in the future.

Currently, we reuse the samples to function like some sort of integration tests. During CI we build the samples which in turn compiles the scss, and we validate if the .css files are generated. This doesn't always work correctlyas the samples don't use the nuget package itself and there was an issue that only occurred with the nuget.

The problem why we haven't created any real unit tests yet is that the main functionality that should be tested are the tasks in msbuild and I don't know how we could unit test that. It might be possible to run the msbuild process in a unit test, but I don't know how to do that and haven't had the time to dig into that. Or maybe we can directly call the CompileSass build task methods and unit test it that way.

@adiessl
Copy link
Contributor Author

adiessl commented May 20, 2022

I would say that testing the MSBuild task would probably only be a final integration test. That could maybe be achieved by deleting all .css files of a test project, then running dotnet build and verifying if the .css files have been generated again.

In order to be able to write unit tests it might be best to extract some code from CompileSass.cs into separate classes and then test these individually – maybe have one class to parse and get the SassCompilerOptions and one class that receives these options and does the actual compilation. Then the execute method of the MSBuild task would basically consist of two lines of code and would not really need a unit test.

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

No branches or pull requests

2 participants