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

Refine test case: nodeset_cmdline, nodeset_runimage, nodeset_shell #3266

Merged
merged 5 commits into from Jun 21, 2017

Conversation

junxiawang
Copy link
Contributor

@junxiawang junxiawang commented Jun 15, 2017

Refine test case: nodeset_cmdline, nodeset_runimage, nodeset_shell #2833
1 Fix issue #2442
2 Fix issue #1871
3 make code clearly for anyone to debug.
During this pull request I modify genesis releated testcase:
1.remove the dependency of perl-xCAT package by xCAT-test
2.add debug information for this testcase to make code clearly for anyone to debug

@@ -71,7 +70,8 @@ BEGIN
print "$::USAGE";
exit 1;
}
my $os = xCAT::Utils->osver("all");
my $os = &get_os;
print "os is $os\n";
Copy link

Choose a reason for hiding this comment

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

What is principle of when using print, when using send_msg?

@@ -264,7 +266,7 @@ sub testxdsh {
if (($value == 1) || ($value == 2) || ($value == 3)) {
`xdsh $noderange -t 2 cat $checkfile |grep $checkstring`;
if ($?) {
foreach (1 .. 1500) {
Copy link

Choose a reason for hiding this comment

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

Why change to 15?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@zet809 I debug the code so change to 15 and will change to 1500

@@ -236,6 +237,7 @@ sub rungenesisimg {
chmod 0755, "/install/my_image/runme.sh";
`tar -zcvf /tmp/my_image.tgz -C /install/my_image .`;
copy("/tmp/my_image.tgz", "/install/my_image") or die "Copy failed: $!";
print "master is $master\n";
Copy link

Choose a reason for hiding this comment

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

Just for debug?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@zet809 yes for debug and will send out message during run testcases

@@ -5,7 +5,6 @@ BEGIN
$::XCATROOT = $ENV{'XCATROOT'} ? $ENV{'XCATROOT'} : -d '/opt/xcat' ? '/opt/xcat' : '/usr';
Copy link

Choose a reason for hiding this comment

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

The line 5 and 7 shall also be removed.

@xuweibj
Copy link

xuweibj commented Jun 16, 2017

Looks good to me.

@whowutwut
Copy link
Member

The testcase:added label in my opinion should be used to track issues that have slipped through regression, we should not use for pull requests. If an issue is opened that slipped through regression testing, we would tag the "testcase:requested" label, then when a pull request is created to add a testcase, we would change "requested" -> "added" and then when we search "testcase:requested" it no longer is needed.

@zet809
Copy link

zet809 commented Jun 16, 2017

Hi, @junxiawang , the pull request looks good to me. But #2833 is used to fix #2442 and #1871, why this duplicated task created?
I will close #2833
And, those 2 issue seems fixed, but @hu-weihua 's concern about "make code clearly for anyone to debug." may not be involved. Need her confirm before merge.

@@ -2,7 +2,7 @@ start:nodeset_shell
description: verify could log in genesis shell
cmd:perl /opt/xcat/share/xcat/tools/autotest/testcase/genesis/genesistest.pl -n $$CN -g
check:rc==0
cmd:perl /opt/xcat/share/xcat/tools/autotest/testcase/genesis/genesistest.pl -n $$CN -s
cmd:perl /opt/xcat/share/xcat/tools/autotest/testcase/genesis/genesistest.pl -n $$CN -s ;if [[ $? -eq 0 ]];then exit 0 ;else cat /var/log/consoles/$$CN; cat /tmp/genesistestlog/*;exit 1;fi

Choose a reason for hiding this comment

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

The console log is very big, include some specific character, and all log of CN in whole automation process , not just the log generated during this case. I don't think dump console log in xcattest log is not a good option.

The other thing is we have pull console log back, it do not need to dump log here.

@hu-weihua
Copy link

I have reviewed the change and agree to merge. Thanks @junxiawang .

@hu-weihua hu-weihua merged commit 998b5ee into xcat2:master Jun 21, 2017
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

5 participants