Skip to content

Commit

Permalink
Prevent accidental manual setting of project name with -s=
Browse files Browse the repository at this point in the history
Before `-s=name` would lead to project name being manually
set to '' rather than using FCM to setup a project name.
This is a correct from the point of view of optparse, but
might seem surprising to users.
  • Loading branch information
wxtim committed Aug 24, 2023
1 parent 6442245 commit cd4de39
Show file tree
Hide file tree
Showing 3 changed files with 54 additions and 1 deletion.
7 changes: 7 additions & 0 deletions CHANGES.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,13 @@ creating a new release entry be sure to copy & paste the span tag with the
updated. Only the first match gets replaced, so it's fine to leave the old
ones in. -->

## __cylc-rose-1.3.1 (<span actions:bind='release-date'>Upcoming</span>)__

### Fixes

[#250](https://github.com/cylc/cylc-rose/pull/250) - Prevent project
name being manually set to an empty string.

## __cylc-rose-1.3.0 (<span actions:bind='release-date'>Released 2023-07-21</span>)__

### Fixes
Expand Down
2 changes: 1 addition & 1 deletion cylc/rose/stem.py
Original file line number Diff line number Diff line change
Expand Up @@ -340,7 +340,7 @@ def _ascertain_project(self, item):
if re.search(r'^\.', item):
item = os.path.abspath(os.path.join(os.getcwd(), item))

if project is not None:
if project:
print(f"[WARN] Forcing project for '{item}' to be '{project}'")
return project, item, item, '', ''

Expand Down
46 changes: 46 additions & 0 deletions tests/unit/test_rose_stem_units.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
import pytest
from pytest import param
from types import SimpleNamespace
from typing import Any, Tuple

from cylc.rose.stem import (
ProjectNotFoundException,
Expand All @@ -34,6 +35,9 @@
from metomi.rose.fs_util import FileSystemUtil


Fixture = Any


class MockPopen:
def __init__(self, mocked_return):
self.mocked_return = mocked_return
Expand Down Expand Up @@ -291,3 +295,45 @@ def test__deduce_mirror():
}
project = 'someproject'
StemRunner._deduce_mirror(source_dict, project)


@pytest.mark.parametrize(
'item, expect, stdout',
(
(
# Normally formed project=url
'foo=bar',
('foo', 'bar'),
"Forcing project for 'bar' to be 'foo'",
),
(
# Malformed project=url
'=foo',
('', 'foo'),
None,
),
)
)
def test_ascertain_project_if_name_supplied(
get_StemRunner: Fixture,
capsys: Fixture,
item: str,
expect: Tuple[str],
stdout: str,
) -> None:
"""Method gives sensible results for different CLI input.
Written because `-s=foo` leads to "item" including a leading = sign.
This led to project name being set to '', and skipping the FCM
calling logic, which the user might expect.
"""
stemrunner = get_StemRunner({})
if stdout:
results = stemrunner._ascertain_project(item)
assert results[:2] == expect
assert stdout in capsys.readouterr().out
else:
with pytest.raises(
ProjectNotFoundException, match='is not a working copy'
):
stemrunner._ascertain_project(item)

0 comments on commit cd4de39

Please sign in to comment.