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 POD documentation for lib/known_bugs #7495

Merged
merged 1 commit into from
May 22, 2019

Conversation

DrMullings
Copy link
Contributor

Adding basic documentation for the known_bugs module

This will be used in main.pm to initialize the backend with that list
To add a known bug simply copy and adapt the following line:
C<push @$serial_failures, {type => soft/hard, message => 'Errormsg', pattern => quotemeta 'ErrorPattern' }>
=cut
Copy link
Member

@SergioAtSUSE SergioAtSUSE May 20, 2019

Choose a reason for hiding this comment

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

For retro compatibility, we need to add a space before cut.

https://perldoc.perl.org/perlpod.html#Hints-for-Writing-Pod

=head1 SYNOPSIS

Allow detection of known errors on the serial console
=cut
Copy link
Member

Choose a reason for hiding this comment

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

For retro compatibility, we need to add a space before cut.

Suggested change
=cut
=cut

@@ -28,12 +28,21 @@ our @EXPORT = qw(
upload_journal
);

=head1 SYNOPSIS
Copy link
Member

@SergioAtSUSE SergioAtSUSE May 20, 2019

Choose a reason for hiding this comment

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

This line is more useful if it contains the name of the module, when the doc is generated apart from the code.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In utils.pm we have this style somehow established, if your intention is to clarify what module we are on, this could be done via the page title?
Where else would you place the synopsis? On an own head2?

C<push @$journal_failures, {type => soft/hard, message => 'Errormsg', pattern => quotemeta 'ErrorPattern' }>
type=soft will force the testmodule result to softfail
type=hard will just emit a soft fail message but the module will do a normal fail
=cut
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
=cut
=cut


Checks the journal for known patterns defined in $journal_failures
Do not touch unless you know what you're doing
=cut
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
=cut
=cut

@asmorodskyi
Copy link
Member

I would add something in general section about our struggle - that on one side we don't want to fail test case when we found something so we would like to go with softfailure but on other side we can't be smart enough to identify some certain bug so we end up breaking rule about softfailure and bug assignment

@DrMullings DrMullings force-pushed the documentation branch 3 times, most recently from 74dfa7c to 4380e63 Compare May 21, 2019 11:56
@DrMullings
Copy link
Contributor Author

Updated according our agreement


=cut

=head2 create_list_of_serial_failures
Copy link
Member

Choose a reason for hiding this comment

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

I am missing the synopsis of the functions. @foursixnine ?

https://github.com/os-autoinst/os-autoinst/blob/f48d4fc7579845f802c0010ec3ca6a9607d9d844/testapi.pm#L135

Suggested change
=head2 create_list_of_serial_failures
=head2 create_list_of_serial_failures
$ref_to_list = create_list_of_serial_failures()

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There is no synopsis, as this function is not intended to be called by the end-user, but only to be extended with new serial failures

Copy link
Member

Choose a reason for hiding this comment

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

I thought this documentation is for the code in general, not only for end-users.

Copy link
Member

Choose a reason for hiding this comment

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

I agree with @SergioAtSUSE's comment. However I'd use it differently, mostly because we should ilustrate the test developer how to interact with this. While it is documented at the os-autoinst/distribution.pm level, extending a bit here, doesn't hurt.

Suggested change
=head2 create_list_of_serial_failures
=head2 create_list_of_serial_failures
my $list = create_list_of_serial_failures();
$testapi::distri->set_expected_serial_failures($list);

Copy link
Member

@SergioAtSUSE SergioAtSUSE May 21, 2019

Choose a reason for hiding this comment

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

but, don't forget the blank line after =head2

Copy link
Member

@SergioAtSUSE SergioAtSUSE left a comment

Choose a reason for hiding this comment

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


=head1 SYNOPSIS

C<use lib/known_bugs>
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
C<use lib/known_bugs>
C<use lib::known_bugs>


=cut

=head2 create_list_of_serial_failures
Copy link
Member

Choose a reason for hiding this comment

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

I agree with @SergioAtSUSE's comment. However I'd use it differently, mostly because we should ilustrate the test developer how to interact with this. While it is documented at the os-autoinst/distribution.pm level, extending a bit here, doesn't hurt.

Suggested change
=head2 create_list_of_serial_failures
=head2 create_list_of_serial_failures
my $list = create_list_of_serial_failures();
$testapi::distri->set_expected_serial_failures($list);


=head2 create_list_of_serial_failures

Creates the list of known bug patterns on the serial logs
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
Creates the list of known bug patterns on the serial logs
Returns the list of known bug patterns on the serial logs

Adding basic documentation for the known_bugs module
@DrMullings
Copy link
Contributor Author

And another update

@asmorodskyi
Copy link
Member

LGTM

@foursixnine foursixnine merged commit 83d042e into os-autoinst:master May 22, 2019
@SergioAtSUSE SergioAtSUSE deleted the documentation branch May 22, 2019 09:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants