tests: don't assume euid != 0 by kyrofa · Pull Request #703 · ros-infrastructure/rosdep

kyrofa · 2019-09-04T21:51:47Z

If tests rely on the environment being a certain way, they should ensure it meets expectations. This PR fixes #702 by updating the tests to ensure they're testing multiple euid conditions, and setting it explicitly where required.

codecov-io · 2019-09-04T22:08:25Z

Codecov Report

Merging #703 into master will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master     #703   +/-   ##
=======================================
  Coverage   75.47%   75.47%           
=======================================
  Files          32       32           
  Lines        2968     2968           
=======================================
  Hits         2240     2240           
  Misses        728      728

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 ab1f06e...67eb9b6. Read the comment docs.

kyrofa · 2019-09-04T22:09:09Z

@nuclearsandwich, @cottsay look sane to you?

nuclearsandwich

Some of the duplication this PR looks like it creates is pre-existing.

Is there a reason that the expected_prefix must be the first argument and why it doesn't have a default value?

kyrofa · 2019-09-19T15:28:24Z

Is there a reason that the expected_prefix must be the first argument

Just because the patch decorators need to go at the end.

[...] and why it doesn't have a default value?

Both for clarity to the caller as well as so we didn't need to use a None canary value and have logic in each test to initialize to an empty list (as a default value of [] is a bad idea). Happy to do that though if you prefer it.

kyrofa · 2019-10-08T16:38:32Z

@nuclearsandwich can I do anything to help this along?

kyrofa · 2019-10-18T20:34:19Z

@nuclearsandwich any further thoughts?

nuclearsandwich · 2019-10-18T20:45:16Z

I've come close to this so many times. Since something is better than nothing, I keep getting hung up on how duplicative this is but I don't have anything better. I need to do a full pass through rosdep PRs. The point release today was to fix an outstanding bug and rather than let it keep longer I just took that.

kyrofa · 2019-10-18T20:50:38Z

I keep getting hung up on how duplicative this is but I don't have anything better

Haha, I hear you. Without a restructuring of the existing tests I'm not sure this can be significantly improved. That said, if you prefer we can change it to only test running as root/non-root once and then mock it out everywhere else to use sudo, which would be a one-line change in the other tests. That's not quite as regression-proof, but the diff would be smaller.

Arnatious · 2019-12-06T10:58:09Z

@nuclearsandwich @kyrofa has #680 invalidated this PR?

kyrofa · 2019-12-06T16:20:57Z

@Arnatious no, it hasn't.

If tests rely on the environment being a certain way, they should ensure it meets its expectations. Update the tests to ensure they're testing multiple euid conditions, and setting it explicitly where required. Fix ros-infrastructure#702 Signed-off-by: Kyle Fazzari <kyle@canonical.com>

nuclearsandwich · 2020-04-02T14:44:11Z

Rebased this against master in order to resolve the merge conflict.

kyrofa · 2020-04-02T16:06:22Z

Thanks @nuclearsandwich!

* Add Suite3 option with Ubuntu Focal (ros-infrastructure#734) Signed-off-by: Shane Loretz <sloretz@openrobotics.org> * [update] Allow to process single ROS distro, fix 723 (ros-infrastructure#738) * [update] skip other distro if --rosdistro passed Signed-off-by: Mikael Arguedas <mikael.arguedas@gmail.com> * ignore argument if specified distro doesnt exist * address review comments * update help message for rosdistro Signed-off-by: Mikael Arguedas <mikael.arguedas@gmail.com> * Fix CI and reduce CI time (ros-infrastructure#739) * test newer python * use yaml.safe_load * pin PyYAML version for Python 3.4 * pass user flag to pip when needed * move slow jobs to the top to reduce CI time Signed-off-by: Mikael Arguedas <mikael.arguedas@gmail.com> * Strip Alpine's patch version from OS codename (ros-infrastructure#716) * Resolve Alpine os_version_type using OsDetect * Reduced Alpine OS VERSION to Major.Minor * improve support for null entries (ros-infrastructure#726) * support null for entire OS and not only OS version syntax like 'ubuntu: null' will now be raising ResolutionError with relevant error message instead of InvalidData with obscure error message will also cover syntax like: " ubuntu: '*': null bionic: [foobar] " Signed-off-by: Mikael Arguedas <mikael.arguedas@gmail.com> Co-authored-by: Scott K Logan <logans@cottsay.net> * Use DNF installer on RHEL 8 and newer (ros-infrastructure#713) RHEL/CentOS 8 uses DNF by default. * Updates to YUM and DNF (ros-infrastructure#640) Output YUM, DNF and RPM versions with `--all-versions` and fix the format guide's default PM for Fedora. * tests: don't assume euid != 0 (ros-infrastructure#703) If tests rely on the environment being a certain way, they should ensure it meets its expectations. Update the tests to ensure they're testing multiple euid conditions, and setting it explicitly where required. Fix ros-infrastructure#702 Signed-off-by: Kyle Fazzari <kyle@canonical.com> * openSUSE package query and install enhancements (ros-infrastructure#729) * Enable PIP installer for openSUSE * openSUSE package detection with RPM capabilities Packages sometimes get renamed and their old name is kept as an rpm capability (like an alias), so the additional flag `--whatprovides` is passed to the `rpm` query. * Fix conditional dependencies when one package uses manifest.xml (ros-infrastructure#737) * Fix conditional dependencies when one package uses manifest.xml Signed-off-by: Shane Loretz <sloretz@osrfoundation.org> * Return rosdeps as a list to avoid breaking the interface. Co-authored-by: Steven! Ragnarök <steven@nuclearsandwich.com> * Guard next(inter) (ros-infrastructure#701) * guard next(inter) Fix ros-infrastructure#691 Signed-off-by: artivis <jeremie.deray@canonical.com> * Handle StopIteration with slightly less line noise. Co-authored-by: Steven! Ragnarök <steven@nuclearsandwich.com> * [Windows] Add console script entry point (ros-infrastructure#656) * Add console script entry point Add console script entry point for platforms (e.g. Windows) not supporting shebang. * remove scripts entry since we are using console_scripts. * fix bad merge. * fix SKIP_PYTHON_SCRIPTS case. Co-authored-by: Lou Amadio <ooeygui@users.noreply.github.com> * Depend on modules packages only to allow co-installability. (ros-infrastructure#750) When the rosdep modules package was split in [ros-infrastructure#731] the module dependencies weren't updated to only depend on modules packages themselves which prevents rosdep modules for python2 and python3 from actually being co-installable. I haven't yet audited to make sure there's no cli usage of these tools in the modules but it's Friday afternoon and I wanted to get this inked before I walked away. When I return to it I'll check for that before proceeding for reviews. * 0.19.0 * also install buildtool_export_depends (ros-infrastructure#753) not adding 'exec_depends' or 'build_export_depends' are both are included inside 'run_depends' Signed-off-by: Mikael Arguedas <mikael.arguedas@gmail.com> * Drop shebang from a non-executable file (ros-infrastructure#755) * add alias for Pop! OS (ros-infrastructure#757) * add alias for Pop! OS Signed-off-by: Mikael Arguedas <mikael.arguedas@gmail.com> Co-authored-by: Shane Loretz <sloretz@openrobotics.org> Co-authored-by: Mikael Arguedas <mikael.arguedas@gmail.com> Co-authored-by: Mark Hedley Jones <MarkHedleyJones@gmail.com> Co-authored-by: Scott K Logan <logans@cottsay.net> Co-authored-by: Kyle Fazzari <kyle@canonical.com> Co-authored-by: Bjar Ne <43565432+gleichdick@users.noreply.github.com> Co-authored-by: Jeremie Deray <deray.jeremie@gmail.com> Co-authored-by: Sean Yen <seanyen@microsoft.com> Co-authored-by: Lou Amadio <ooeygui@users.noreply.github.com>

Arnatious approved these changes Sep 4, 2019

View reviewed changes

nuclearsandwich reviewed Sep 19, 2019

View reviewed changes

nuclearsandwich self-assigned this Apr 2, 2020

nuclearsandwich approved these changes Apr 2, 2020

View reviewed changes

nuclearsandwich force-pushed the bugfix/702/tests_assume_euid_ne_0 branch from 67eb9b6 to fdd89e2 Compare April 2, 2020 14:43

nuclearsandwich merged commit cfa21c8 into ros-infrastructure:master Apr 2, 2020

kyrofa deleted the bugfix/702/tests_assume_euid_ne_0 branch April 2, 2020 16:49

kyrofa mentioned this pull request Apr 2, 2020

ROS 2 Foxy Fitzroy ros2/ros2#830

Closed

3 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

tests: don't assume euid != 0#703

tests: don't assume euid != 0#703
nuclearsandwich merged 1 commit intoros-infrastructure:masterfrom
kyrofa:bugfix/702/tests_assume_euid_ne_0

kyrofa commented Sep 4, 2019 •

edited

Loading

Uh oh!

codecov-io commented Sep 4, 2019 •

edited

Loading

Uh oh!

kyrofa commented Sep 4, 2019

Uh oh!

nuclearsandwich left a comment

Uh oh!

kyrofa commented Sep 19, 2019 •

edited

Loading

Uh oh!

kyrofa commented Oct 8, 2019

Uh oh!

kyrofa commented Oct 18, 2019

Uh oh!

nuclearsandwich commented Oct 18, 2019

Uh oh!

kyrofa commented Oct 18, 2019 •

edited

Loading

Uh oh!

Arnatious commented Dec 6, 2019

Uh oh!

kyrofa commented Dec 6, 2019 •

edited

Loading

Uh oh!

nuclearsandwich commented Apr 2, 2020

Uh oh!

kyrofa commented Apr 2, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

kyrofa commented Sep 4, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov-io commented Sep 4, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

kyrofa commented Sep 4, 2019

Uh oh!

nuclearsandwich left a comment

Choose a reason for hiding this comment

Uh oh!

kyrofa commented Sep 19, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kyrofa commented Oct 8, 2019

Uh oh!

kyrofa commented Oct 18, 2019

Uh oh!

nuclearsandwich commented Oct 18, 2019

Uh oh!

kyrofa commented Oct 18, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Arnatious commented Dec 6, 2019

Uh oh!

kyrofa commented Dec 6, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nuclearsandwich commented Apr 2, 2020

Uh oh!

kyrofa commented Apr 2, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

kyrofa commented Sep 4, 2019 •

edited

Loading

codecov-io commented Sep 4, 2019 •

edited

Loading

kyrofa commented Sep 19, 2019 •

edited

Loading

kyrofa commented Oct 18, 2019 •

edited

Loading

kyrofa commented Dec 6, 2019 •

edited

Loading