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 more perl signatures to lib::OpenQA::Files and Git #4490

Merged
merged 2 commits into from Feb 1, 2022

Conversation

FruitFly638
Copy link
Contributor

Adds more perl signatures to lib/OpenQA/Files.pm and lib/OpenQA/Git.pm

https://progress.opensuse.org/issues/105127

@okurz okurz changed the title Add more perl signatuers to lib::OpenQA::Files and Git Add more perl signatures to lib::OpenQA::Files and Git Feb 1, 2022
Copy link
Member

@okurz okurz left a comment

Choose a reason for hiding this comment

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

looks ok besides test failures:

[10:00:34] t/16-utils-runcmd.t ....................................... 1/? 
    #   Failed test 'OpenQA::Git thows an exception if parameter missing'
    #   at t/16-utils-runcmd.t line 46.
    # expecting: Regexp ((?^:no user specified))
    # found: Too few arguments for subroutine 'OpenQA::Git::commit' at t/16-utils-runcmd.t line 44.
    # Looks like you failed 1 test of 3.

#   Failed test 'make git commit (error handling)'
#   at t/16-utils-runcmd.t line 56.
Too many arguments for subroutine 'OpenQA::Git::enabled' at t/16-utils-runcmd.t line 85

and

[09:58:31] t/ui/12-needle-edit.t ...................... 1/? 
    #   Failed test 'highlight appears correct'
    #   at t/ui/12-needle-edit.t line 369.
    #          got: 'Fatal error when saving needle.
    # Task for saving needles failed: Too few arguments for subroutine 'OpenQA::Git::set_to_latest_master' at /home/squamata/project/lib/OpenQA/Task/Needle/Save.pm line 106.'
    #     expected: 'Needle test-newneedle created/updated - restart job'

    #   Failed test 'test-newneedle.json created'
    #   at t/ui/12-needle-edit.t line 375.

    #   Failed test 'test-newneedle.png created'
    #   at t/ui/12-needle-edit.t line 376.

    #   Failed test 'commit message matches'
    #   at t/ui/12-needle-edit.t line 378.
    #                   undef
    #     doesn't match '(?^us:Example.*\n.*Multi.*\n.*line)'

    #   Failed test 'files added to commit'
    #   at t/ui/12-needle-edit.t line 379.
    #     Structures begin differing at:
    #          $got->{add} = Does not exist
    #     $expected->{add} = ARRAY(0x559609f2d518)
    # 0

    #   Failed test 'overwrite dialog shown'
    #   at t/ui/12-needle-edit.t line 236.
    #          got: '0'
    #     expected: '1'

    #   Failed test 'Needle part of the title'
    #   at t/ui/12-needle-edit.t line 237.
    #          got: ''
    #     expected: 'Sure to overwrite test-newneedle?'
    # Looks like you failed 7 tests of 34.
[09:58:31] t/ui/12-needle-edit.t ...................... 3/? 
#   Failed test 'Create new needle'
#   at t/ui/12-needle-edit.t line 395.
clickElement: element not interactable at /home/squamata/project/t/ui/../lib/OpenQA/SeleniumTest.pm:80 at /home/squamata/project/t/ui/../lib/OpenQA/SeleniumTest.pm line 83.
	OpenQA::SeleniumTest::__ANON__(Test::Selenium::Chrome=HASH(0x559608a3cef8), "Error while executing command: clickElement: element not inte"..., HASH(0x559609f7e518)) called at /usr/lib/perl5/vendor_perl/5.26.1/Selenium/Remote/Driver.pm line 356
	Selenium::Remote::Driver::catch {...} ("Error while executing command: clickElement: element not inte"...) called at /usr/lib/perl5/vendor_perl/5.26.1/Try/Tiny.pm line 123
	Try::Tiny::try(CODE(0x559609f3d0e8), Try::Tiny::Catch=REF(0x559609f67248)) called at /usr/lib/perl5/vendor_perl/5.26.1/Selenium/Remote/Driver.pm line 361
	Selenium::Remote::Driver::__ANON__(CODE(0x559608685a70), Test::Selenium::Chrome=HASH(0x559608a3cef8), HASH(0x559609f7e518)) called at (eval 1702)[/usr/lib/perl5/vendor_perl/5.26.1/Class/Method/Modifiers.pm:89] line 1
	Selenium::Remote::Driver::__ANON__(Test::Selenium::Chrome=HASH(0x559608a3cef8), HASH(0x559609f7e518)) called at (eval 1704)[/usr/lib/perl5/vendor_perl/5.26.1/Class/Method/Modifiers.pm:148] line 2
	Selenium::Remote::Driver::_execute_command(Test::Selenium::Chrome=HASH(0x559608a3cef8), HASH(0x559609f7e518)) called at (eval 1663)[/usr/lib/perl5/vendor_perl/5.26.1/Sub/Quote.pm:3] line 17
	Selenium::Remote::WebElement::_execute_command(Test::Selenium::Remote::WebElement=HASH(0x559609f73bc8), HASH(0x559609f7e518)) called at /usr/lib/perl5/vendor_perl/5.26.1/Selenium/Remote/WebElement.pm line 63
	Selenium::Remote::WebElement::click(Test::Selenium::Remote::WebElement=HASH(0x559609f73bc8)) called at t/ui/12-needle-edit.t line 243
	main::overwrite_needle("test-newneedle") called at t/ui/12-needle-edit.t line 390
	main::__ANON__() called at /usr/lib/perl5/5.26.1/Test/Builder.pm line 309
	eval {...} called at /usr/lib/perl5/5.26.1/Test/Builder.pm line 309
	Test::Builder::subtest(Test::Builder=HASH(0x5595fd612870), "Create new needle", CODE(0x559609ee5628)) called at /usr/lib/perl5/5.26.1/Test/More.pm line 807
	Test::More::subtest("Create new needle", CODE(0x559609ee5628)) called at t/ui/12-needle-edit.t line 395
[09:58:31] t/ui/12-needle-edit.t ...................... 4/? # Tests were run but no plan was declared and done_testing() was not seen.
                                                             [09:58:31] t/ui/12-needle-edit.t ...................... Dubious, test returned 1 (wstat 256, 0x100)
Failed 1/4 subtests

I recommend to run at least the tests for the according files locally before creating pull requests or create draft pull requests.

@FruitFly638
Copy link
Contributor Author

I fixed the previous problems, I'm not sure if I should add $args = undef in all the other subroutines that didn't fail the tests too, though.

@codecov
Copy link

codecov bot commented Feb 1, 2022

Codecov Report

Merging #4490 (768ce57) into master (18c7f1a) will not change coverage.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #4490   +/-   ##
=======================================
  Coverage   97.94%   97.94%           
=======================================
  Files         374      374           
  Lines       33992    33992           
=======================================
  Hits        33295    33295           
  Misses        697      697           
Impacted Files Coverage Δ
lib/OpenQA/Files.pm 100.00% <100.00%> (ø)
lib/OpenQA/Git.pm 95.91% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 18c7f1a...768ce57. Read the comment docs.

sub _prepare_git_command {
my ($self, $args) = @_;

sub _prepare_git_command ($self, $args = undef) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I know you added $args = undef here because the tests failed. However, it looks still wrong because if $args can be undef we should not unconditionally access a key of it like it is done in the next line. Since we're actually intentionally passing undef here in some cases you've found a bug. See my other inline comment for an easy fix.

lib/OpenQA/Git.pm Show resolved Hide resolved
Copy link
Contributor

@Martchus Martchus left a comment

Choose a reason for hiding this comment

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

See comment by @perlpunk

@mergify mergify bot merged commit 0a1802f into os-autoinst:master Feb 1, 2022
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.

None yet

4 participants