anaconda / ae5-tools

A command-line tool for scripting AE5 actions
https://www.anaconda.com/enterprise/
BSD 3-Clause "New" or "Revised" License
9 stars 8 forks source link

Minor docstring and unit test fixes #140

Closed jlstevens closed 2 years ago

jlstevens commented 2 years ago

This is a minor PR that improves some help text but is mainly an excuse to see whether tests are still passing on master (in order to make progress in other PRs such as #139 where tests are currently failing in strange ways).

jlstevens commented 2 years ago

As I suspected, something must have changed in the ecosystem/environment: the last commit on master went green but now tests fail on windows and the ubuntu build just seems to hang!

At least it is now clear that CI fixes are needed independent of the other outstanding PRs (which will then need to be rebased).

jlstevens commented 2 years ago

The failing windows tests at least have been explained: the AE5 instance being used is now behind a firewall. I'm still not sure why the ubtuntu-latest builds fail to even start but for the time being I will continue work by running the tests locally.

mcg1969 commented 2 years ago

Note that the test suite itself is marked complete. I think this is a GitHub actions UI issue. I think the behavior is intended, that the ubuntu tests are interrupted because the Windows tests failed, but the way it's indicated in the logs is confusing.

On Tue, Jan 25, 2022 at 7:44 AM Jean-Luc Stevens @.***> wrote:

The failing windows tests at least have been explained: the AE5 instance being used is now behind a firewall. I'm still not sure why the ubtuntu-latest builds fail to even start but for the time being I will continue work by running the tests locally.

— Reply to this email directly, view it on GitHub https://github.com/Anaconda-Platform/ae5-tools/pull/140#issuecomment-1021196538, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAVLBJ7VMJETSS3RONNCQ4TUX2SLTANCNFSM5MYGHSQQ . You are receiving this because you are subscribed to this thread.Message ID: @.***>

--

Michael C. Grant, Ph.D.Vice President, ServicesAnaconda, Inc. http://anaconda.com

Convex optimization resources:CVX http://cvxr.com/cvx/, TFOCS http://cvxr.com/tfocs/, CVX Forum http://ask.cvxr.com/ CVX Research, Inc. http://cvxr.com/ http://anaconda.com http://cvxr.com

jlstevens commented 2 years ago

That does clarify things!

I would have expected the ubuntu tests to get cancelled given they are sequenced after the windows tests (and conditional on those passing) instead of getting stuck in a 'starting' state. At any rate, I'll revert my last commit given that everything is now explained (and I have gotten the tests running locally!).

mcg1969 commented 2 years ago

Yeah, I blame GitHub for confusing indicators here. In a fail-fast situation, it should provide a different status for jobs that have been skipped.

jlstevens commented 2 years ago

When running the tests locally, the test_login_time unit tests were failing. When I looked into it, I saw that the 'lastLogin' timestamp is compared to the time now (in UTC) and that the former timestamp was one hour ahead of UTC (causing the tests to fail). That happens to be the current time where I am running the tests locally.

Looking at the output of ae5 user list confirms that the last login time is not UTC. As a result I believe that assert ltm1 < now will have passed in any timezone west of Greenwich (modulo daylight savings time) and failed east of the Greenwich meridian!

I've now updated the two relevant unit tests to use now = datetime.now() instead.