Skip to content
This repository was archived by the owner on Oct 12, 2022. It is now read-only.
/ druntime Public archive

Comments

Don't run main after unittests#1724

Closed
andralex wants to merge 1 commit intodlang:masterfrom
andralex:noMainAfterUnittests
Closed

Don't run main after unittests#1724
andralex wants to merge 1 commit intodlang:masterfrom
andralex:noMainAfterUnittests

Conversation

@andralex
Copy link
Member

This change in behavior may disrupt some folks but we need to push things forward. It makes no sense to run the program after unittests.

@andralex
Copy link
Member Author

I suspect now everybody will love the current behavior :)

@andralex
Copy link
Member Author

@wilzbach
Copy link
Contributor

There is also prior art: #1685

@MartinNowak
Copy link
Member

Please figure out a way that does not break every D project out there!

One example, add a runtime switch --DRT-stop_after_tests=, and ask people to explicitly make a choice before switching the default. Maybe you could also figure out how to allow a choice for Runtime.moduleUnitTester - multiple declarations.
Another way might be to not insist on a main function (might be tricky).

Also this needs to be coordinated with what dub and dub test do.

@WalterBright
Copy link
Member

Yeah, it's a good change, but I worry about breaking code and if this is really worth it.

@schveiguy
Copy link
Member

#1685 does a similar thing in a backwards compatible way.

@adamdruppe
Copy link
Contributor

Does anybody seriously care about this kind of "breakage"? Virtually everyone complains about and has numerous hacks to bypass the existing behavior toward what this does, and it is trivial to handle.

@mihails-strasuns
Copy link
Contributor

So here we have 2 approaches. One uses boring engineering and stable deprecation path. Other throws in few lines of dirty code and allows to make smartass remarks about own superiority. Both provide same desired functionality. Which one to chose?

This is bullshit - trying to argument breaking change with "oh it doesn't really affect anyone" when the same outcome can be achieved without making such assumption. I could possibly understand such attitude if change was much needed and migration path was plain impossible. But here the choice is simply between doing things right and not being able to pass an opportunity to spit in my face.

@mihails-strasuns
Copy link
Contributor

#1685 combined with --DRT flag to affect behaviour of default test runner sounds like a good way to address it.

@adamdruppe
Copy link
Contributor

This is bullshit

Amen. The other PR (or one of the ones before it, this isn't new) should have been merged without argument a long time ago. We have a chance to make that right: just merge it and close this, or merge this and close it.

Someone just make an executive decision. This isn't worth arguing over and it is so frustrating to see this pattern over and over again, someone does the work, it gets ignored, someone else does the work again, it gets debated pointlessly, forgotten again... then later on someone is guaranteed to say "hey contributors, want change? start doing great work". Now that is spit in the face.

Just pick a PR, I don't care which, and merge it. Close the other one immediately. Then you can write your --DRT switch patch on top of it.

@andralex
Copy link
Member Author

All: not to worry, this was just to start at the upper bound of changing everything silently and see how much is liable to break.

@wilzbach @schveiguy cool! didn't know about that work, it's pretty much how I intended to continue this. For backwards compatibility, I'm thinking we could just add a flag -unittest-only that when used instead of -unittest does not run main. Then we can phase off -unittest if necessary.

@adamdruppe the executive decision is we should move forward with this, at best with @schveiguy's work.

@Dicebot could we please tone down the language? Thanks.

Merry Christmas!

@andralex
Copy link
Member Author

Closing this to shift focus on #1685

@andralex andralex closed this Dec 25, 2016
@adamdruppe
Copy link
Contributor

@andralex splendid! Let's see more decisive action taking concrete steps in 2017, I have high hopes for that year.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants