-
Notifications
You must be signed in to change notification settings - Fork 21
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
test: Add github actions for Windows #62
Conversation
67fe9ed
to
d898506
Compare
Signed-off-by: Robert Luttrell <[email protected]>
d898506
to
d41d035
Compare
def test_uses_worker_settings( | ||
self, | ||
posix_job_user_setting: str | None, | ||
expected_config_posix_job_user: PosixSessionUser | None, | ||
parsed_args: ParsedCommandLineArguments, | ||
mock_worker_settings_cls: MagicMock, | ||
) -> None: | ||
"""Tests that any parsed_cli_args without a value of None are passed as kwargs when | ||
creating a WorkerSettings instance""" | ||
|
||
# GIVEN | ||
mock_worker_settings_cls.side_effect = None | ||
mock_worker_settings: MagicMock = mock_worker_settings_cls.return_value | ||
mock_worker_settings.posix_job_user = posix_job_user_setting | ||
|
||
# Needed because MagicMock does not support gt/lt comparison | ||
mock_worker_settings.host_metrics_logging_interval_seconds = 10 | ||
|
||
# WHEN | ||
config = config_mod.Configuration(parsed_cli_args=parsed_args) | ||
|
||
# THEN | ||
# Assert that the attributes are taken from the WorkerSettings instance | ||
assert config.farm_id is mock_worker_settings.farm_id | ||
assert config.fleet_id is mock_worker_settings.fleet_id | ||
assert config.profile is mock_worker_settings.profile | ||
assert config.verbose is mock_worker_settings.verbose | ||
assert config.no_shutdown is mock_worker_settings.no_shutdown | ||
assert config.allow_instance_profile is mock_worker_settings.allow_instance_profile | ||
assert ( | ||
config.cleanup_session_user_processes | ||
is mock_worker_settings.cleanup_session_user_processes | ||
) | ||
assert config.capabilities is mock_worker_settings.capabilities | ||
assert config.impersonation.inactive == (not mock_worker_settings.impersonation) | ||
if expected_config_posix_job_user: | ||
assert isinstance(config.impersonation.posix_job_user, PosixSessionUser) | ||
assert ( | ||
config.impersonation.posix_job_user.group | ||
== expected_config_posix_job_user.group | ||
) | ||
assert ( | ||
config.impersonation.posix_job_user.user == expected_config_posix_job_user.user | ||
) | ||
else: | ||
assert config.impersonation.posix_job_user is None | ||
assert config.worker_logs_dir is mock_worker_settings.worker_logs_dir | ||
assert config.local_session_logs is mock_worker_settings.local_session_logs | ||
assert config.worker_persistence_dir is mock_worker_settings.worker_persistence_dir | ||
assert ( | ||
config.worker_credentials_dir | ||
is mock_worker_settings.worker_persistence_dir / "credentials" | ||
) | ||
assert config.host_metrics_logging is mock_worker_settings.host_metrics_logging | ||
assert ( | ||
config.host_metrics_logging_interval_seconds | ||
is mock_worker_settings.host_metrics_logging_interval_seconds | ||
) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The diff here is only indentation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A portion of this PR overlaps with #35.
I would very much prefer for that to be merged first.
Will wait on that change to be merged then rebase |
What was the problem/requirement? (What/Why)
We need Github actions to run code quality on Windows to support Windows development.
What was the solution? (How)
Add Windows OS to the code quality workflow.
xfail
andskipif
tests that are currently failing/erroring on Windows. These will be addressed in the future.What is the impact of this change?
Code quality check runs on Windows in addition to the already-existing macOS/ubuntu jobs.
How was this change tested?
Ran the CI
Was this change documented?
No
Is this a breaking change?
No