Skip to content

Comments

enable assertions in CI tests#7344

Closed
MartinNowak wants to merge 1 commit intodlang:masterfrom
MartinNowak:assert_tests
Closed

enable assertions in CI tests#7344
MartinNowak wants to merge 1 commit intodlang:masterfrom
MartinNowak:assert_tests

Conversation

@MartinNowak
Copy link
Member

@MartinNowak MartinNowak commented Nov 21, 2017

I guess we'll still need to fix a couple of things and might have to disable class invariants to reduce the overhead.

@dlang-bot
Copy link
Contributor

dlang-bot commented Nov 21, 2017

Thanks for your pull request, @MartinNowak!

Bugzilla references

Your PR doesn't reference any Bugzilla issue.

If your PR contains non-trivial changes, please reference a Bugzilla issue or create a manual changelog.

Testing this PR locally

If you don't have a local development environment setup, you can use Digger to test this PR:

dub fetch digger
dub run digger -- build "master + dmd#7344"

@WalterBright
Copy link
Member

As I recall, back when (Kenji?) tried it it substantially slowed down the compiler. I want to review the best way forward on this first. I think the non-release mode inserts too many checks.

@MartinNowak
Copy link
Member Author

MartinNowak commented Nov 21, 2017

As I recall, back when (Kenji?) tried it it substantially slowed down the compiler. I want to review the best way forward on this first. I think the non-release mode inserts too many checks.

One way forward is -release=assert,invariant so that we can enable asserts but not invariants, for the time being we might be better off with the slow-down.

@mathias-lang-sociomantic
Copy link
Contributor

so that we can enable asserts but not invariants

Shameless self-plug: https://github.com/dlang/DIPs/blob/master/DIPs/DIP1006.md

@wilzbach
Copy link
Contributor

I like the idea of finally moving forward with DIP1006! BTW one minor thing: if we start to use a different compiler build (non-release) than the one we ship on the auto-tester, how do we make sure that the version we ship doesn't get accidentally broken?

@ibuclaw
Copy link
Member

ibuclaw commented Dec 11, 2017

Guess you can close #6746 then.

@wilzbach
Copy link
Contributor

wilzbach commented Jan 4, 2018

for the time being we might be better off with the slow-down.

Yes, we could simply do this on a faster CI (e.g. SemaphoreCI is twice as fast as Travis - I checked this recently. Compare SemaphoreCi build, Travis build).

I guess we'll still need to fix a couple of things and might have to disable class invariants to reduce the overhead.

FYI: our CircleCi build already runs without ENABLE_RELEASE - though

@ibuclaw
Copy link
Member

ibuclaw commented Jan 5, 2018

Yes, we could simply do this on a faster CI (e.g. SemaphoreCI is twice as fast as Travis

I would say more. On a good day, gdc is built with testsuite and unittests ran in 14 minutes flat on semaphore.

Travis barely could do the same in 50 minutes.

@MartinNowak
Copy link
Member Author

What I found implementing #7980, we could hack this already today.
-release -unittest disables all run-time checks but assertions. If we versioned out dmd's few unittests, we'd get what we want for CI.

@ibuclaw
Copy link
Member

ibuclaw commented Mar 11, 2018

What I found implementing #7980, we could hack this already today.
-release -unittest disables all run-time checks but assertions.

Heh, wouldn't that be a bug? Or maybe not. It does seem surprising though.

@MoonlightSentinel
Copy link
Contributor

FWIW CircleCI and Azure already run the test suite with debug builds. Is this still required?

@Geod24
Copy link
Member

Geod24 commented Jul 2, 2020

Don't think so. Also we should have it in the release, but that's another topic.

@Geod24 Geod24 closed this Jul 2, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

9 participants