Make ct a behaviour for test suites (original by richcarl)

[paulo-ferraz-oliveira]

This PR is related to #2000 (I used the same title and all), and continues from #2000 (comment).

[paulo-ferraz-oliveira]

OK, so here's the story:

1. found Make ct a behaviour for test suites #2000 while searching for a Common Test suite behaviour-defining module,
2. asked if I could help,
3. was given an OK to help,
4. asked for guidance on how to proceed on top of previous work,
5. got the required guidance,
6. started working on the current PR - if all goes well this will supersede Make ct a behaviour for test suites #2000 and it can be closed.

[paulo-ferraz-oliveira]

I proceeded to:

1. cherry pick from the original,
2. moving the existing code to lib/common_test/src/ct_suite.erl,
3. improving the -spec(_).s by adding missing elements and going deeper in specification,
4. formatting that code as per other elements in Erlang/OTP,
5. tweaking exported elements.

[paulo-ferraz-oliveira]

What's missing:
1. knowing if this module is named correctly and placed where it should be placed,
2. knowing if there's any other type we should be exporting,
3. knowing if type naming is correct,
4. knowing if this change involves writing tests - Edit: "apply it to some highly developed application such as ssl as a proof of concept."
5. knowing if this is going to maint or master,
6. understanding how I should proceed to add documentation (the missing man page) - is there an example you can point me to that's similar to your expectations?

Edit: all of these moved to the main description element.

[paulo-ferraz-oliveira]

Could you guide me as per the questions in #2794 (comment)? Thanks.

[IngelaAndin]

You can look at gen_statem and ssh_server_channel as examples of callback behaviours documentation using specs. Some of the documentation in the common_test module (that is the current documentation of a test suite callbacks) is of course relevant but it can be improved and better structured with help of the usage of specs to generate parts of the documentation.

[paulo-ferraz-oliveira]

You can look at gen_statem and ssh_server_channel as examples of callback behaviours documentation using specs. Some of the documentation in the common_test module (that is the current documentation of a test suite callbacks) is of course relevant but it can be improved and better structured with help of the usage of specs to generate parts of the documentation.

Thank you, @IngelaAndin.

Can I assume that:

1. module ct_suite is named correctly and placed where it should be?
2. there's no other type we should be exporting?
3. type naming is correct?
4. this is going to maint?

About testing
Does this change involve writing tests - or can I, otherwise, apply the newly added behaviour to already-existing test elements - all suites, for example?

[paulo-ferraz-oliveira]

@IngelaAndin, do you think it would be OK to document the Data Types in common_test.html, like dict.html does, for example?

The main difference is that:

• dict types are exported by dict:, but
• ct_suite types aren't exported by common_test:, as no such module exists.

Probably we could document this in the "header" but I see no example like it in the source docs.

[IngelaAndin]

Can I assume that:

1. module ct_suite is named correctly and placed where it should be?

Yes

2. there's no other type we should be exporting?

Our policy is to be restrictive with exporting types. Only input and return types should be necessary to export.

3. type naming is correct?

I think that might still be up for discussion, but it will be simpler to discuss it when we have a built documentation proposal and not to difficult to change.

4. this is going to maint?

Yes we will accept it for maint.

It does not require any special tests. It might be challenging to apply it to all OTP test suites as there might still be legacy things
around in a lot of test suites dated before common_test. You could perhaps apply it to some highly developed application such as ssl as a proof of concept.

[IngelaAndin]

When it comes to the data types, they should be documented in the same fashion as in dict, this you should get almost for free when using specs to generate the docs. We usually put the spec in the relevant Erlang file an not in a a header file. So you can export the relevant types from ct_suite.erl The common_test.xml should not have any documentation for the callbacks this
module ought to only have information about possible application environment variables, it is originally a man(6) information and is often called _app.xml

The xml will need something like this (a not full lexample from ssl.xml) :

<datatypes>
    <datatype_title>Types used in TLS/DTLS</datatype_title>

    <datatype>
      <name name="socket"/>
    </datatype>
    
    <datatype>
      <name name="sslsocket"/>
      <desc>
	<p>An opaque reference to the TLS/DTLS connection, may be used for equality matching.</p>
	</desc>
    </datatype>

    <datatype>
      <name name="tls_option"/>
    </datatype>
        
    <datatype>
      <name name="tls_client_option"/>
    </datatype>
    
    <datatype>
      <name name="tls_server_option"/>
    </datatype>
   

[paulo-ferraz-oliveira]

when using specs to generate the docs

How do you use "specs to generate the docs"? I'm used to edoc but haven't found any documentation on how to do it for Erlang/OTP base. I looked at:

• https://github.com/erlang/otp/blob/master/CONTRIBUTING.md
• https://github.com/erlang/otp/wiki/Documentation
  but haven't run the appropriate commands yet.

[paulo-ferraz-oliveira]

The common_test.xml should not have any documentation for the callbacks this
module ought to only have information about possible application environment variables, it is originally a man(6) information and is often called _app.xml

I'm a bit confused here, since there's no common_test.xml and types are exported from ct_suite.erl.

I think for the doc. to be clear:

1. common_test_app.xml shall be fixed (as per the issues I found before - missing types, for example),
2. a Data Types section must be added somewhere (I don't know if the correct place is common_test_app.xml),
3. there should be some indication that the behavior is implemented by module ct_suite.

[IngelaAndin]

We do not use edoc. We want you to create a manual page ct_suite.xml that includes the data types and documentation for the ct_suite behaviour. The xml from code above refers to data types exported from ssl.erl and creates the datatype part of the manual page.

You can refer to types with seetype: (examle from gen_statem)

<func>
      <name since="OTP 19.0">Module:format_status(Opt, [PDict,State,Data]) ->
        Status
      </name>
      <fsummary>
	Describe the current <c>gen_statem</c> status (optional).
      </fsummary>
      <type>
        <v>Opt = normal | terminate</v>
        <v>PDict = [{Key, Value}]</v>
	<v>
	  State =
	  <seetype marker="#state">state()</seetype>
	</v>
	<v>
	  Data =
	  <seetype marker="#data">data()</seetype>
	</v>

Alas I do not think it is publicly documented at the moment.

If we should remove common_test.xml or perhaps replace it with common_test_app.xml is an other issue. I think we can address that once we are getting ct_suite.xml in place.

[paulo-ferraz-oliveira]

Hm... OK.

I made some changes to common_test_app.xml to approach it to whatever is in ct_suite.erl but if the callbacks are to be described in ct_suite I guess much of common_test_app.xml will be a repetition of that. I'll add the module and then we'll proceed (I also updated the main description so that I can keep track of what's missing).

[IngelaAndin]

The documentation existing of the callback API today in common_test.xml is not at all in a place that we usually would expect it to be. I think it has some legacy reasons for being so. The origin of common_test was a very non common test framework that ended up becoming a common test framework replacing the old OTP test framework, instead of being used for an other not so common use case. I think the relevant part should be moved to ct_suite.xml

[paulo-ferraz-oliveira]

not at all in a place that we usually would expect it to be

Hadn't understood this.

I think the relevant part should be moved to ct_suite.xml

Sure, that's why I included that in the initial description. In any case I made the changes in the existing man page so it's easier to validate once I copy-paste them (since I will mostly copy-paste).

[paulo-ferraz-oliveira]

I tried to address all documentation issues in the most recent commits:

• 6f400fa: add ct_suite
• 4f9bcef: use seetype references

I'm still unable to generate the documentation locally (keep getting make errors) but I'll continue trying to solve it.
Edit: I'm rebasing on top of maint since I'm getting undef errors and strange stacktraces trying to generate the docs.

[paulo-ferraz-oliveira]

I'm looking into this again (thanks, @IngelaAndin, will try your fix in a minute). I hope there's no conflict (since macOS in case-insensitive for filenames) between ct_SUITE (already exists) and ct_suite (the new file, being added).

[paulo-ferraz-oliveira]

I get the same error!

I think you need

...

Thanks for the help. I hope 6fbd047 does it. It's not quite what you wrote, but I assume you assumed I hadn't created an issue before (which I might have - and which, hopefully, this commit fixes).

[paulo-ferraz-oliveira]

@IngelaAndin: regarding "some types aren't linked, like ct_group_props() (is probably mis-use of <datatypes>)", could you help me? The HTML file is generated with broken links, but this is surely due to my lack of knowledge regarding the use of datatypes. The datatypes I'm using also refer to some internal ones (that don't get "generated"). Is there a way to have them "generated" inline?

e.g.

ct_test_def() :: ct_testname() | ct_group_ref() | ct_testcase_ref()

shouldn't have references to type ct_group_ref(), for example, since that one's internal (as in "not exported"). Is there a way to have it expanded in the doc.? Should the "generator" know about "not exported" and behave differently (this would be the subject of a different issue, of course).

[paulo-ferraz-oliveira]

I solved the compilation issues, along with the test issues.
I think the documentation generation issues are solved.
I'd like validation on the solution chosen for the hyperlinks (datatypes refer to the functions that, themselves, describe the types).

[paulo-ferraz-oliveira]

After this, I'd like to know if you feel there's something missing.

[IngelaAndin]

The documentation still does not build, some links needs fixing. If you do git grep "common_test#" in common_test/doc/src and "query replace" the result with ct_suite# I think it should then work and you should then have done the structural improvement of making a behaviour for the test suites and further improvements to the actual text should be scheduled into our backlog.

[IngelaAndin]

We are aware that you can not currently see which types are exported and hence may be refereed to in other specs and which are only sub types that are for documentation purposes. There is no obvious solution at the moment, time will tell but this is as you say an other issue than the one of this PR.

[paulo-ferraz-oliveira]

We are aware that you can not currently see which types are exported and hence may be refereed to in other specs and which are only sub types that are for documentation purposes. There is no obvious solution at the moment, time will tell but this is as you say an other issue than the one of this PR.

I kind of hacked a solution for this:

1. in the datatypes section I wrote e.g. ct_info(): the return type for :suite(), and then
2. in the :suite() part of the doc. I refer to the type ct_info() as containing other sub-types (and they're all in that context).

[paulo-ferraz-oliveira]

The documentation still does not build, some links needs fixing. If you do git grep "common_test#" in common_test/doc/src and "query replace" the result with ct_suite# I think it should then work and you should then have done the structural improvement of making a behaviour for the test suites and further improvements to the actual text should be scheduled into our backlog.

I'll do this :) I just keep piling up issues haha. (shouldn't the build process, locally, complain about that, though?)

[IngelaAndin]

I think it is the link checker that finds the problem, so it is a later step.

[paulo-ferraz-oliveira]

I think it is the link checker that finds the problem, so it is a later step.

If only there was a make check-doc target 😄

Is anything missing, @IngelaAndin? I want to help further if further help is required.

[paulo-ferraz-oliveira]

@peterdmv, is there something missing that I can do, still?

[paulo-ferraz-oliveira]

Had an open review (by mistake). Tried to post something to delete it afterward; couldn't. Sorry about the noise.

[paulo-ferraz-oliveira]

Thanks for this and your guidance.