Rez test improvements (dev mode, interactive flag) #759

fnaum · 2019-10-08T21:40:56Z

Some rez test improvements

added the ability to run rez test in "developer mode" using rez test . from the root folder of the package.
Automatically build the package if the package is not found in the REZ_LOCAL_PACKAGES_PATH
Update the built package.py if the tests section changed
added --interactive option, to resolve an interactive environment to run the tests. ie. (rez test --interactive TESTNAME). In case the package has multiple variants, you can also specify the variant using --variant
By default change to a build/rez_test_result directory to run the test and store the test artifacts (this way we can pick those results from tools like sonar, that expect the nose and coverage reports in a known place). If they need to be run on the root of the packages run_in_root can be set in the tests field of the package.py.
added --clean to clean the test artifacts created from a previous test run.
Tests added along with the mock module under vendor

related to #665

Introduced support for test env activation from command line. New flag --env set the test environment which will be sourced (base on test definitions in package.py. Support for anonymous tests invocation. You can ommit the package name from the test command 'rez test <package_name>' while running tests from the package root directory. Added auto build in case package is not built before tests rez/vendor: ----------- - added mock 2.0.0 - added funcsigs 1.0.2

Recently introduced improvement in rez test command was causing error in case when rez test command has been executed with package version eg. rez test package==1.0.0 This change repairs that situation.

This change introduces a new field in the package configuration called "run_in_root", which is optional and by default set to False. Setting it to True will store test artifacts in the package root * Review requested changes

nerdvegas · 2019-11-15T04:21:29Z

Update: Hoping to be working on this soon, we need the same functionality at Method. Stay tuned.

nerdvegas · 2019-11-15T23:35:24Z

src/rez/cli/test.py

    PKG_action = parser.add_argument(
        "--extra-packages", nargs='+', metavar="PKG",
        help="extra packages to add to test environment")
    PKG_action = parser.add_argument(
-        "PKG",
+        "PKG", nargs='?',


-h does not make it clear what it means if PKG is not provided. It also is not clear from the help that PKG can be a path (ie .). I'm tempted to say that we keep it simpler than this - PKG is either provided or not, and if not, then we're in "developer" mode.

nerdvegas · 2019-11-15T23:40:11Z

src/rez/cli/test.py

+        help="resolves an interactive environment for running the tests.")
+    parser.add_argument(
+        "--variant", dest="variant", default=0, type=int,
+        help="variant number which should be used to set the interactive env. Works only with --interactive option")


This should work in the general case also, it makes sense to specify a variant to run tests on.

nerdvegas · 2019-11-15T23:41:44Z

src/rez/cli/test.py

+
+    Returns: True or False
+    """
+    return not name or name == '.'


Too fragile. As mentioned earlier, I think it'd be better if developer mode is enabled only if PKG is not provided. Right now is odd because PKG can be a package string, or ".", but not any other valid path. There is too much variability in what PKG can be.

nerdvegas · 2019-11-15T23:42:31Z

src/rez/cli/test.py

+    Args:
+        path: path to the package
+
+    Returns: Tuple of values (package_name, is_dev_run)


Wrong return docstring. Also not sure we need this function.

probably not needed, I'm guessing was added to make the code more testable, but I'm pretty sure you can call directly get_developer_package

nerdvegas · 2019-11-15T23:45:17Z

src/rez/cli/test.py

+        return None
+
+
+def prepare_dev_env_package(package):


I'm actually pretty hesitant about rez-test possibly implicitly installing a package like this. Package installation is always explicit everywhere else. Furthermore this feels arbitrary - the package gets installed if it wasn't there, but what if it's there but put of date due to a sourcecode change? We can't detect that and I don't think we should.

It seems simpler and less error prone to simply give an error if the current developer package isn't present as a local package, and leave it up to the user if and when to rebuild/reinstall the package.

yes, we can not detect that. ATM we are only detecting changes on the packages.py and reinstalling the package when the tests are out of date.
I'll do a poll tomorrow when I'm at work, I'm guessing not many devs are really relying on this feature. So, I guess it can go away

nerdvegas · 2019-11-15T23:49:16Z

src/rez/cli/test.py

+    sys.argv = original_args
+
+
+def update_package_in_local_packages_path(package, installed_package):


Again this feels arbitrary - the package is being kept up-to-date wrt its tests, but not its actual payload (ie if source was changed). I think we should just leave it up to the developer to reinstall the package.

Having said that, something that I think would be acceptable in developer mode is if the test definitions are always taken from the developer package, rather than from the installed package.

In that case, if they are taken from the developer package, I think people will be happy

nerdvegas · 2019-11-15T23:56:34Z

src/rez/package_test.py

-                stdout=self.stdout,
-                stderr=self.stderr,
-                block=True)
+            test_result_dir = os.path.abspath('build/rez_test_result' if not test_info["run_in_root"] else '.')


'build/rez_test_result' should be configurable. As mentioned earlier, we should have a tests_artifact_path setting rather than run_in_root. Then, to run tests in root for a given package, you do a package-level config override, like so:

with scope("config") as c: c.tests_artifact_repository = '.'

This is more consistent with how settings are managed in other parts of the codebase.

nerdvegas · 2019-11-15T23:59:13Z

src/rez/utils/system.py

+
+
+@contextmanager
+def change_dir(dir_path):


This is very similar to existing rez.utils.filesystem.retain_cwd. I would update this existing func to be able to chdir to a specified path, and use that instead.

nerdvegas · 2019-11-16T00:04:54Z

So I'm freed up to work on this next week for Method. I think it would make sense to use this PR and #758 as a basis for that work, rather than updating these PRs though.

Reasons are:

I need to make immediate use of the time I'm getting from Method (rez dev time doesn't happen often!)
We need to make these changes with REP-001: Various rez-tests enhancements #665 in mind, so there will probably be some significant differences to these PRs.

Fede I wanted to let you know in case you thought I'm throwing this work away! Rest assured a chunk of the work is already done here and I'll be incorporating it into the upcoming PRs. Expect to see more next week.

nerdvegas · 2019-12-03T03:34:01Z

Note that some of this PR has been superseded by #807. However, I'm still open to adding the dev mode functionality shown here, as long as there's consensus. It's not clear to me how this should work exactly - as mentioned above, the way the package is automatically re-installed if the tests definition changes, feels inconsistent.

Wrt --interactive, it definitely makes sense to do this, and I'm aiming to make this available in a separate PR which mimics the behaviour in this PR.

JeanChristopheMorinPerso · 2019-12-04T03:58:58Z

@nerdvegas @fnaum The way I see the dev mode is just to avoid some typing. So instead of doing rez-build -ci && rez-test packageA==x.x.x, you could just do rez-build -ci && rez-test .. It would make things simpler to use IMO. So it's just a "shorthand".

nerdvegas · 2019-12-04T04:10:03Z

Yeah I don't mind that functionality, but in #759 'dev mode' is not quite that simple, as it does things like automatically re-install the package if its tests change. I am not so sure about that, because it feels a bit arbitrary as to whether a package is reinstalled (ie yes if tests change, no if anything else changes). In #759, test byproducts are also written to a specific directory when in dev mode. So the question is also whether this should happen and if so, what that path should be. A

…

On Wed, Dec 4, 2019 at 2:58 PM Jean-Christophe Morin < ***@***.***> wrote: @nerdvegas <https://github.com/nerdvegas> @fnaum <https://github.com/fnaum> The way I see the dev mode is just to avoid some typing. So instead of doing rez-build -ci && rez-test packageA==x.x.x, you could just do rez-build -ci && rez-test .. It would make things simpler to use IMO. So it's just a "shorthand". — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#759>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAMOUSSJGRR7JWFQNGQ3RE3QW4TIHANCNFSM4I6XOR4Q> .

JeanChristopheMorinPerso · 2019-12-04T04:22:05Z

in #759 'dev mode' is not quite that
simple, as it does things like automatically re-install the package if its
tests change.

Yeah, I wouldn't go with the auto-install. Hence why I was giving rez-build -ci && rez-test . as an example. I'd almost prefer if the user was manually building (as he probably knows more what he needs to do for his tests to run then us).

In #759, test byproducts are also written to a specific directory when in
dev mode. So the question is also whether this should happen and if so,
what that path should be.

I still haven't made my mind on this. To be frank, the first time I reviewed this PR I didn't like the idea that rez would know where my tests artifacts are. And as of today I have the same feeling. You could have 5 different test target that each run different tools that will all generate tests artifacts in a different way (and sometimes if would even be impossible to choose where to put them). That means each test would have to define it's artifact directory. Though a possible case, it might not be a super common one. It would be easy to implement though. I think the hardest part will be to agree on how to handle that...

nerdvegas · 2020-01-20T23:53:16Z

Marking as ON HOLD - these features are desired (interactive mode, dev mode), however the code has changed too much for this PR to be viable. Keeping for reference until an equivalent is merged.

fnaum · 2020-01-23T10:34:08Z

Sorry, I was not active when you reviewed this, we lost the only resource that was working with rez and all the other tools and we have not found a replacement yet. :(
Feel free to close it at any time

Wojtek Biniek and others added 6 commits October 8, 2019 18:14

Fix for failing rez release process during test stage

efc247d

Recently introduced improvement in rez test command was causing error in case when rez test command has been executed with package version eg. rez test package==1.0.0 This change repairs that situation.

Fix for tested package version

bf60dd7

Run tests outside current folder (build/rez_test_result) )

04c56a6

This change introduces a new field in the package configuration called "run_in_root", which is optional and by default set to False. Setting it to True will store test artifacts in the package root * Review requested changes

fix merge issues

fc8bffd

Rename --env with --interactive. Fix some print statements

129c813

fnaum requested review from nerdvegas and JeanChristopheMorinPerso October 8, 2019 21:40

fnaum mentioned this pull request Oct 28, 2019

Rez test release hook #758

Closed

nerdvegas reviewed Nov 15, 2019

View reviewed changes

nerdvegas changed the title ~~Rez test improvements~~ Rez test improvements (dev mode, interactive flag) Nov 21, 2019

nerdvegas mentioned this pull request Dec 3, 2019

Rep001 1 (rez-test improvements) #807

Merged

nerdvegas added the ON HOLD Awaiting other tasks, or kept for reference only label Jan 20, 2020

ColinKennedy mentioned this pull request Feb 21, 2022

rez-test --interactive flag #1224

Open

maxnbk requested a review from a team as a code owner November 15, 2022 23:58

JeanChristopheMorinPerso requested a review from a team as a code owner April 4, 2023 15:30

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Rez test improvements (dev mode, interactive flag) #759

Rez test improvements (dev mode, interactive flag) #759

fnaum commented Oct 8, 2019 •

edited

Loading

nerdvegas commented Nov 15, 2019

nerdvegas Nov 15, 2019

nerdvegas Nov 15, 2019

nerdvegas Nov 15, 2019

fnaum Jan 23, 2020

nerdvegas Nov 15, 2019

fnaum Jan 23, 2020

nerdvegas Nov 15, 2019

fnaum Jan 23, 2020

nerdvegas Nov 15, 2019

fnaum Jan 23, 2020

nerdvegas Nov 15, 2019

fnaum Jan 23, 2020

nerdvegas Nov 15, 2019

nerdvegas commented Nov 16, 2019

nerdvegas commented Dec 3, 2019

JeanChristopheMorinPerso commented Dec 4, 2019

nerdvegas commented Dec 4, 2019 via email

JeanChristopheMorinPerso commented Dec 4, 2019

nerdvegas commented Jan 20, 2020

fnaum commented Jan 23, 2020 •

edited

Loading

		sys.argv = original_args


		def update_package_in_local_packages_path(package, installed_package):

Rez test improvements (dev mode, interactive flag) #759

Are you sure you want to change the base?

Rez test improvements (dev mode, interactive flag) #759

Conversation

fnaum commented Oct 8, 2019 • edited Loading

nerdvegas commented Nov 15, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nerdvegas commented Nov 16, 2019

nerdvegas commented Dec 3, 2019

JeanChristopheMorinPerso commented Dec 4, 2019

nerdvegas commented Dec 4, 2019 via email

JeanChristopheMorinPerso commented Dec 4, 2019

nerdvegas commented Jan 20, 2020

fnaum commented Jan 23, 2020 • edited Loading

fnaum commented Oct 8, 2019 •

edited

Loading

fnaum commented Jan 23, 2020 •

edited

Loading