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

Allow same group name within different parent groups #1507

Merged
merged 1 commit into from
Dec 1, 2017

Conversation

mitiao
Copy link
Contributor

@mitiao mitiao commented Nov 27, 2017

see: https://progress.opensuse.org/issues/17846
feedback and advices please

@coolo
Copy link
Contributor

coolo commented Nov 27, 2017

please add a test case to verify we can have now groups with different parents but same name.

@mitiao mitiao changed the title Allow same group name within different parent groups [WIP] Allow same group name within different parent groups Nov 27, 2017
@Martchus
Copy link
Contributor

Martchus commented Nov 28, 2017

This will likely work, as far as I know the name is really just used for display purposes. But yes, you should provide a test. Just add a few more lines in the tests we already have, eg. t/ui/14-dashboard-parents.t. You can use the schema_hook function to add required fixtures.

@mitiao
Copy link
Contributor Author

mitiao commented Nov 29, 2017

@Martchus thanks for your advice.
I am working on the test with a chromedriver issue.
Run whatever test with selenium, for example by prove -l -v t/ui/14-dashboard-parents.t and log_chromedriver gave the error:
[61.063][INFO]: RESPONSE InitSession unknown error: Chrome failed to start: exited abnormally
any idea?

@Martchus
Copy link
Contributor

You could use PhantomJS instead of Chromium. Just ensure the SELENIUM_CHROME environment variable is not set and it should use PhantomJS.

Note that I'm using Chromium and not Chrome. As far as I remember, I had to install the driver through a separate package under Tumbleweed. So maybe just a package missing?

@coolo
Copy link
Contributor

coolo commented Nov 29, 2017

phantomjs support is dead - as IS SELENIUM_CHROME

@Martchus
Copy link
Contributor

Martchus commented Nov 29, 2017

I'm sometimes still using it and it works (beside on particular line in one particular test). So it is worth a try as a workaround. Beside, things might get better when next PhantomJS version is released. It will use Qt WebKit ng/revived (or whatever they want to call it now).

@codecov
Copy link

codecov bot commented Dec 1, 2017

Codecov Report

Merging #1507 into master will increase coverage by 0.11%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1507      +/-   ##
==========================================
+ Coverage    88.2%   88.31%   +0.11%     
==========================================
  Files         106      106              
  Lines        8173     8149      -24     
==========================================
- Hits         7209     7197      -12     
+ Misses        964      952      -12
Impacted Files Coverage Δ
lib/OpenQA/Schema.pm 100% <ø> (ø) ⬆️
lib/OpenQA/Schema/Result/JobGroups.pm 100% <ø> (ø) ⬆️
lib/OpenQA/Worker.pm 96.55% <0%> (-3.45%) ⬇️
lib/OpenQA/Worker/Common.pm 82.35% <0%> (-1.84%) ⬇️
lib/OpenQA/Resource/Locks.pm 92.85% <0%> (-0.48%) ⬇️
lib/OpenQA/Worker/Jobs.pm 82.96% <0%> (-0.29%) ⬇️
lib/OpenQA/Utils.pm 92.32% <0%> (-0.26%) ⬇️
lib/OpenQA/Worker/Cache.pm 86.11% <0%> (-0.26%) ⬇️
lib/OpenQA/WebAPI/Controller/API/V1/Locks.pm 96.82% <0%> (-0.1%) ⬇️
lib/OpenQA/Schema/Result/Jobs.pm 90.92% <0%> (-0.02%) ⬇️
... and 7 more

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 9d7e532...50a9d10. Read the comment docs.

@codecov
Copy link

codecov bot commented Dec 1, 2017

Codecov Report

Merging #1507 into master will decrease coverage by 14.21%.
The diff coverage is 80%.

Impacted file tree graph

@@             Coverage Diff             @@
##           master    #1507       +/-   ##
===========================================
- Coverage   87.37%   73.15%   -14.22%     
===========================================
  Files         106       96       -10     
  Lines        8173     7682      -491     
===========================================
- Hits         7141     5620     -1521     
- Misses       1032     2062     +1030
Impacted Files Coverage Δ
lib/OpenQA/Schema/Result/JobGroups.pm 27.41% <ø> (-72.59%) ⬇️
lib/OpenQA/Schema.pm 89.33% <ø> (-10.67%) ⬇️
lib/OpenQA/WebAPI/Controller/API/V1/JobGroup.pm 51.28% <80%> (-45.78%) ⬇️
lib/db_profiler.pm 12% <0%> (-88%) ⬇️
lib/OpenQA/Resource/Locks.pm 9.33% <0%> (-84%) ⬇️
lib/OpenQA/Schema/Result/Assets.pm 9.63% <0%> (-74.1%) ⬇️
lib/OpenQA/Schema/Result/Screenshots.pm 7.69% <0%> (-67%) ⬇️
lib/OpenQA/Schema/ResultSet/JobSettings.pm 13.33% <0%> (-61.67%) ⬇️
lib/db_helpers.pm 50% <0%> (-50%) ⬇️
... and 49 more

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 b2fe214...96ffa3b. Read the comment docs.

@mitiao mitiao changed the title [WIP] Allow same group name within different parent groups Allow same group name within different parent groups Dec 1, 2017

return 0 if $self->is_parent;
my $properties = $self->load_properties;
return $self->resultset->search({name => $properties->{'name'}, parent_id => undef});
Copy link
Contributor

Choose a reason for hiding this comment

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

JobGroup.pm: Hash key with quotes at line 102, column 60. Avoid useless quotes. (Severity: 5)

@coolo coolo merged commit 359a8b2 into os-autoinst:master Dec 1, 2017
@coolo
Copy link
Contributor

coolo commented Dec 1, 2017

the test cases are problematic at the moment, but the ui tests passed, so we should be fine

coolo pushed a commit that referenced this pull request Dec 1, 2017
commit 359a8b2
Author:     Wei Jiang <mitiao@gmail.com>
AuthorDate: Sat Dec 2 02:56:27 2017 +0800
Commit:     Stephan Kulow <stephan@kulow.org>
CommitDate: Fri Dec 1 19:56:27 2017 +0100

    Allow same group name within different parent groups (#1507)

    See: https://progress.opensuse.org/issues/17846
@mitiao mitiao deleted the dev branch December 4, 2017 01:58
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

3 participants