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

add ct-suite template #2072

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

add ct-suite template #2072

wants to merge 1 commit into from

Conversation

tsloughter
Copy link
Collaborator

This adds a message that can be printed after the template is applied. This is most useful in the init-release template I'll open a PR for soon because updating rebar.config isn't possible so it'll print out what to add to rebar.config for relx.

@@ -0,0 +1,6 @@
{description, "Common Test suite module"}.
{variables, [
{name, "myapp", "Name of the Common Test suite to create"}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

myapp points what served as a copy-paste source ;-)

-include_lib("common_test/include/ct.hrl").
-include_lib("stdlib/include/assert.hrl").

-compile(export_all).
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Quite controversial discussion may start with this one.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you decide to keep export_all is it better to have -compile([export_all, nowarn_export_all]). instead of different directives?

Copy link
Contributor

@max-au max-au left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does it even make sense to have half-minimalistic template?
By "half", I mean that in general Common Test requires all/0 to be exported, and nothing else. So minimalistic design means nothing else to be added, otherwise more extended template I suggested could serve a better job.

@tsloughter
Copy link
Collaborator Author

I would think a suite with only all() and the test functions are rare so having the inits is useful and very little to delete if not used. Large comment blocks just make it harder to see everything :).

Based this on what I'd want to use, so yea, personal opinion.

@tsloughter tsloughter changed the title add ct-suite template and support for usage message add ct-suite template Aug 31, 2020
@tsloughter tsloughter requested a review from ferd August 31, 2020 17:20
[].

all() ->
[].
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably not important, but indentation seems off here (5 v. 4).

[].

init_per_suite(Config) ->
Config.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

... and here too (3 v. 4).

suite() ->
[].

all() ->

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Including a dummy test would be good here 👌🏼

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

Successfully merging this pull request may close these issues.

4 participants