Fix content app tests not running on macOS.

[jchristgit]

macOS uses /var/... as its temp directory, causing issues with the hardcoded
usage of /tmp as the temporary directory. As /tmp is also used in some
tests (for now), population of the directory is done regardless.

I have also attempted completely removing the /tmp path in tests in favour of
the platform-specific temporary directory, however, with the current
implementation if directory traversal for wiki content, this is currently more
complicated to implement (in the tests) than this solution.

[MarkKoz]

Is it feasible to remove this and instead always populate the platform-specific dir? You'd have to change any uses of /tmp to self.os_tmpname but the tests should have access to that attribute without any trouble.

[jchristgit]

Yes, well, sort of. I've tried doing so, but it start looking into subfolders of the platform-specific directory. As mac has a nesting of folders in /var/, this crashes in test_get_context_data_for_category_with_page:

$ python manage.py test --no-input --force-color pydis_site/apps/content/tests/
System check identified no issues (0 silenced).
.E..................
======================================================================
ERROR: test_get_context_data_for_category_with_page (pydis_site.apps.content.tests.test_views.PageOrCategoryViewTests)
Make sure the proper page is returned for category locations with pages.
----------------------------------------------------------------------
Traceback (most recent call last):
  File "~/workspace/pydis-site/pydis_site/apps/content/tests/test_views.py", line 156, in test_get_context_data_for_category_with_page
    self.ViewClass.dispatch(request, location=self.os_tmpname)
  File "~/workspace/pydis-site/pydis_site/apps/content/views/page_category.py", line 26, in dispatch
    return super().dispatch(request, *args, **kwargs)
  File "~/venv-packages/site-packages/django/views/generic/base.py", line 97, in dispatch
    return handler(request, *args, **kwargs)
  File "~/venv-packages/site-packages/django/views/generic/base.py", line 158, in get
    context = self.get_context_data(**kwargs)
  File "~/workspace/pydis-site/pydis_site/apps/content/views/page_category.py", line 59, in get_context_data
    entry_info["name"] = utils.get_category(entry)["title"]
  File "~/workspace/pydis-site/pydis_site/apps/content/utils.py", line 16, in get_category
    return yaml.safe_load(path.joinpath("_info.yml").read_text(encoding="utf-8"))
  File "~/venv-packages/site-packages/pyfakefs/fake_pathlib.py", line 540, in read_text
    with FakeFileOpen(self.filesystem)(self._path(), mode='r',
  File "~/venv-packages/site-packages/pyfakefs/fake_filesystem.py", line 5202, in __call__
    return self.call(*args, **kwargs)
  File "~/venv-packages/site-packages/pyfakefs/fake_filesystem.py", line 5263, in call
    file_object = self._init_file_object(file_object,
  File "~/venv-packages/site-packages/pyfakefs/fake_filesystem.py", line 5318, in _init_file_object
    self.filesystem.raise_os_error(errno.ENOENT, file_path)
  File "~/venv-packages/site-packages/pyfakefs/fake_filesystem.py", line 1010, in raise_os_error
    raise OSError(errno, message, filename)
FileNotFoundError: [Errno 2] No such file or directory in the fake filesystem: 'var/folders/_info.yml'

----------------------------------------------------------------------

and I'm not entirely sure how to deal with it yet. I'm still investigating.

[MarkKoz]

Doesn't that mean that the populate function doesn't correctly populate that directory? In such case, this starts to feel a bit like a hack since the data is just good enough to not fail despite technically not being correct.

[MarkKoz]

And what about your idea to use a subdirectory as the root?

[jchristgit]

Yes, but also no.

The problem is that when you make it populate the subfolder and the subfolder only (e.g. the result of tempfile.gettempdir()), it starts breaking because it expects an _info.yml in the top-level directory:

    ...
FileNotFoundError: [Errno 2] No such file or directory in the fake filesystem: '/var/_info.yml'
    ...
FileNotFoundError: [Errno 2] No such file or directory in the fake filesystem: 'var/_info.yml'

... so there doesn't seem to be a "one size fits all" here. Unless, of course, we move all content within a subdirectory, but that currently appears to be the most work, especially since the /tmp directory contents are also tested.

[jchristgit]

That said, I'd also be open to just skipping the test when tempfile.gettempdir() != '/tmp'.

[MarkKoz]

that currently appears to be the most work

What does it involve? I was under the impression you'd just have to prefix all the paths used in tests with the subdir name.

That said, I'd also be open to just skipping the test when

Not sure, but I think it's preferable to run all tests if possible, and it is with the current solution; it's just the most ideal solution.

[jchristgit]

What does it involve? I was under the impression you'd just have to prefix all the paths used in tests with the subdir name.

Yes, that's correct - more specifically, os_tmpname is just adjusted to be the complete path obtained from tempfile.gettempdir().

That said, I expect that moving into the temporary file directory to create the files in subdirectories and below should be a bit more involved, especially as the logic of the tests would need to be updated accordingly. That also being said, it's half past midnight here, so maybe I'm missing some obvious solution here.

Not sure, but I think it's preferable to run all tests if possible, and it is with the current solution; it's just the most ideal solution.

I will assume that you meant "it's just not the most ideal solution" here and I would agree, but I am not sure what a better implementation might look like.

We could perhaps create files in the temporary directory altogether, remove pyfakefs, and skip assumptions about directory layout (e.g. /tmp). That said, I think a change like that would be better suited for a later pull request, as right now I'm mainly trying to get my local tests to work :(

[MarkKoz]

I will assume that you meant "it's just not the most ideal solution"

Yes, I did. Sorry.

That said, I expect that moving into the temporary file directory to create the files in subdirectories and below should be a bit more involved, especially as the logic of the tests would need to be updated accordingly.

I don't see why it would be more involved. It all seems straightforward to me, and there aren't even that many tests. I'll try to play around with it.

[coveralls]

Coverage remained the same at 100.0% when pulling 7c9f81b on fix-content-app-tests-on-macos into c11c06e on main.

[MarkKoz]

Okay, that wasn't too bad. Let me know what you think of this approach. BTW we should probably just squash this PR when merging.

[jchristgit]

Yes, that looks good! +1 to squashing.

# Set the module constant within Patcher to use the fake filesystem# https://jmcgeheeiv.github.io/pyfakefs/master/usage.html#modules-to-reloadwith fake_filesystem_unittest.Patcher() as _:    BASE_PATH = Path("res")

This code specifically was the missing piece for me.

I've tested this on my Mac and it works fine. Thank you!

[kosayoda]

Thanks for reporting and @MarkKoz for writing the fix. I don't have macOS so I can only test it on my machine but it works fine.

[Xithrius]

This was tested on my linux machine as well, works as expected.