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

Better shell specific testing #1136

Conversation

davidlatwe
Copy link
Contributor

  • fix rez-env output test is not testing on specified shell #1135,

  • also, able to exclude specific shells from testing with @per_available_shell(). This is currently as a helper feature for working on test_rez_env_output.

    Example usage as follow:

        @per_available_shell(exclude=["cmd", "powershell"])
        @install_dependent()
        def test_rez_env_output(self):
            ...

Comment on lines 199 to 200
if sh_out[1]:
raise Exception("Command failed:\n%s" % sh_out[1])
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I didn't use process.returncode is because, it seems that powershell always return 0 even the command actually failed.

Copy link
Contributor

Choose a reason for hiding this comment

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

Isn't this a case of powershell -File vs. -Command?
Only the latter passes the error code.
https://stackoverflow.com/questions/50200325/returning-an-exit-code-from-a-powershell-script

Or maybe $ErrorActionPreference = "Stop":
https://stackoverflow.com/questions/9948517/how-to-stop-a-powershell-script-on-the-first-error

The checks also need to happen form $LastExitCode I believe:
https://stackoverflow.com/questions/10666035/difference-between-and-lastexitcode-in-powershell

What an over-engineered little shell :)

@davidlatwe
Copy link
Contributor Author

davidlatwe commented Sep 29, 2021

Hmmm, looks like checking stderr isn't a good idea ? test_rez_env_output failed on csh shell :(

Edit
Ah, that should be because the csh isn't the default shell on Ubuntu and Mac, as powershell isn't the defaule shell on Windows, they all got flushed out after rez-env --shell flag is used !

txt = '!hey>$'

    def _test(txt):
        # Assumes that the shell has an echo command, build-in or alias
        binpath = os.path.join(system.rez_bin_path, "rez-env")
        args = [binpath, "--shell", target_shell, "--", "echo", txt]
    
        process = subprocess.Popen(
            args, stdin=subprocess.PIPE, stdout=subprocess.PIPE,
            stderr=subprocess.PIPE, universal_newlines=True
        )
        sh_out = process.communicate()
        if sh_out[1]:
>           raise Exception("Command failed:\n%s" % sh_out[1])
E           Exception: Command failed:
E           hey: Event not found.
E            (in shell 'csh')

@davidlatwe
Copy link
Contributor Author

Github action cannot find any runner that matches ubuntu-16.04, due to :
https://github.blog/changelog/2021-04-29-github-actions-ubuntu-16-04-lts-virtual-environment-will-be-removed-on-september-20-2021/

Ubuntu version bumped in test matrix.

@nerdvegas nerdvegas merged commit 3f672d9 into AcademySoftwareFoundation:master Oct 11, 2021
@davidlatwe davidlatwe deleted the issue_1135-better-shell-specific-testing branch October 12, 2021 08:42
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.

rez-env output test is not testing on specified shell
3 participants