dub icon indicating copy to clipboard operation
dub copied to clipboard

`dub test` ignores `unittest` block sections for the module containing `main`

Open Luhrel opened this issue 3 years ago • 5 comments

System information

  • dub version: 1.29.0
  • OS Platform and distribution: Arch Linux x86_64
  • compiler version: DMD v2.100.0, LDC 1.29.0 (based on DMD v2.099.1 and LLVM 14.0.6)

Bug Description

dub test ignores unittest block sections for the module containing the main function, if another module exists.

How to reproduce?

  • Execute the command dub init dub_test_issue, leave all fields by default.
  • Go inside the project : cd dub_test_issue.
  • Set the content of the file source/app.d to the following code :
import std.stdio;

void main()
{
	writeln("Edit source/app.d to start your project.");
}

unittest
{
	assert(0);
}
  • Run dub test and notice that the test fails as excepted.
  • Add a new module : > source/empty_module.d
  • Re-run dub test and notice that the test does not fail.

The bug doesn't occur when running dub -b unittest instead of dub test.

Expected Behavior

The last step of the previous chapter should fail ; dub test should run the unit tests in the module containing the main function. According to the documentation :

Builds the package and executes all contained unit tests.

External links

In 2017, another user has met this issue (discussion)

Luhrel avatar Jul 21 '22 10:07 Luhrel

there was #1248 which attempted to fix this, but has been closed: https://github.com/dlang/dub/pull/1248#issuecomment-1158717403

@Geod24 would you say we should close this for the same reason as well?

Right now the default app.d implicitly gets assigned to the mainSourceFile and then there are 2 cases how this can behave:

  • with targeType = auto (the default) the app.d is excluded because the library is being tested and not the application, so no tests are executed there
  • with explicit targetType = executable, DUB outputs a warning Excluding main source file source/app.d from test.

WebFreak001 avatar Jul 28 '22 13:07 WebFreak001

Related: https://github.com/dlang/dub/issues/270

I think that the main reason app.d was being excluded before was because druntime used to run the main after the unittests, as explained in the comment. Now that it's not the case anymore, we should rethink how we approach this, which is why the PR was closed.

My main concern with the approach of using a configuration for unittest is that it does not scale well. As your number of configurations grows, so will the number of test target.

Another concern is the "dub test runner". IMO there shouldn't be any special handling in dub w.r.t. unittests. All the magic should be in druntime. -checkaction=context should be the default, and the druntime-generated binary should allow you to re-run a specific test / give you a rich output.

Coming back to the issue: The approach I've been taking recently is to have configuration == binary (or lib). So having a mainSourceFile here means you're generating an executable, and that executable is exclusive to that configuration (IDK if you remember that discussion?).

I would leave this issue open as it's an issue with DUB, even if it might depend on a fix in another component (druntime). What do you think ?

Geod24 avatar Jul 28 '22 13:07 Geod24

I think we should just start to include the main source file in tests now, at least for a potential DUB 2.0

WebFreak001 avatar Jul 28 '22 14:07 WebFreak001

Agreed (it wasn't clear, but that's what I implied with removing the unittest configuration).

Geod24 avatar Jul 28 '22 14:07 Geod24