"Examples expected to fail, but not failing"

[smarie]

https://github.com/smarie/mkdocs-gallery/actions/runs/1951764926

Examples expected to fail, but not failing:
/home/runner/work/mkdocs-gallery/mkdocs-gallery/docs/examples/no_output/plot_syntaxerror.py
/home/runner/work/mkdocs-gallery/mkdocs-gallery/docs/examples/no_output/plot_raise.py
Please remove these examples from 'expected_failing_examples' in your mkdocs.yml file.

It seems related to expected failures in the gallery ?

[smarie]

Note: it is maybe related to the last pull request merged #27 ?

In particular maybe the examples fail the first time, but the second time the hash is already computed and therefore the execution is skipped. @mchaaler would you like to check this hypothesis ?

In that case we should maybe change the mkdocs build happening after the test execution in the test nox session at https://github.com/smarie/mkdocs-gallery/blob/main/noxfile.py#L125 : we should probably add a second one after the first one. That way, this scenario of re-executing with hash files already there would be covered too.

[smarie]

Note: I yanked 0.7.4 on PyPi repo, so it is not available anymore to the general audience (except for those explicitly specifying mkdocs-gallery==0.7.4).

[mchaaler]

In particular maybe the examples fail the first time, but the second time the hash is already computed and therefore the execution is skipped. @mchaaler would you like to check this hypothesis ?

Indeed this seems possible, since only a few modifications were made since the last successful build.
The thing is that I don't understand why I cannot reproduce the bug in a 'standard' second mkdocs build execution.

[smarie]

I think I got it: we are both on windows and the issue appears on linux.

mkdocs-gallery/src/mkdocs_gallery/gen_gallery.py

Lines 804 to 806 in 2acaff3

	passing_unexpectedly = [
	src_file for src_file in passing_unexpectedly
	if re.search(gallery_conf.get('filename_pattern'), src_file)]

on my machine once a first run of mkdocs build has been made, on the second run just before this code is called, I have the correct issue :

>> passing_unexpectedly
{'C:/_dev/python_ws/_Libs_OpenSource/mkdocs-gallery/docs/examples/no_output/plot_raise.py',
 'C:/_dev/python_ws/_Libs_OpenSource/mkdocs-gallery/docs/examples/no_output/plot_syntaxerror.py'}

>> gallery_conf.get('filename_pattern')
'\\\\plot'

However the re.search(gallery_conf.get('filename_pattern'), src_file) never returns True and therefore passing_unexpectedly becomes empty.

What bugs me is that the code is the same than in is_executable_example..

mkdocs-gallery/src/mkdocs_gallery/gen_data_model.py

Lines 180 to 193 in c3f2389

	def is_executable_example(self) -> bool:
	"""Tell if this script has to be executed according to gallery configuration: filename_pattern and global plot_gallery
	This can be false for a local module (not matching the filename pattern),
	or if the gallery_conf['plot_gallery'] is set to False to accelerate the build (disabling all executions)
	Returns
	-------
	is_executable_example : bool
	True if script has to be executed
	"""
	filename_pattern = self.gallery_conf.get('filename_pattern')
	execute = re.search(filename_pattern, str(self.src_py_file)) and self.gallery_conf['plot_gallery']
	return execute

This is probably related to full vs. relative paths ?

[smarie]

Ok it seems that this is due to the fact that

• in is_executable_example, self.src_py_file is a pathlib Path object and therefore its string representation contains backslashes, therefore matching the filename_pattern
• in _parse_failures the paths returned by _expected_failing_examples are not pathlib objects and therefore have a different string representation.

I see a few actions from this point on

• first, have _parse_failures and is_executable_example rely on the same new util function matches_filename_pattern so that if we modify it later, it is consistent. This method should assert that it receives pathlib objects
• second, investigate why _parse_failures manipulates non-pathlib objects since they should originally have been collected as pathlib objects. It would be sad to re-convert them back
• finally, the bug from ci should be reproducible on windows. Then we can try to fix it :)

I open a ticket #36 containing the first two steps

[mchaaler]

I think I got it: we are both on windows and the issue appears on linux.

I was just about to write something very similar about Windows/linux and the _parse_failures method which empties the list when it should not because re.search(gallery_conf.get('filename_pattern'), src_file) returns None...

Tried to wrap everything up from work before leaving, but was not able to to tie it up in time.

[smarie]

No worries of course ! Let me know (no hurry)

[mchaaler]

First step should be adressed by PR#39 now

[mchaaler]

• in _parse_failures the paths returned by _expected_failing_examples are not pathlib objects and therefore have a different string representation.

It seems that the Path.as_posix() method tends to be kind of overused in the code base: this leads to converting a lot of Path objects to strings, with the side effects we are currently facing.

This is/was the case in gen_gallery._expected_failing_examples():

mkdocs-gallery/src/mkdocs_gallery/gen_gallery.py

Lines 788 to 791 in 7f39f9e

	def _expected_failing_examples(gallery_conf: Dict, mkdocs_conf: Dict) -> Set[str]:
	"""The set of expected failing examples"""
	return set((Path(mkdocs_conf['docs_dir']) / path).as_posix()
	for path in gallery_conf['expected_failing_examples'])

A quick search returns about 35 hits across all files in the project. Most of them are probably necessary, but some might lead to unexpected mismatch between the source file and the filename_pattern.

[smarie]

Indeed. This is because when I migrated from sphinx-gallery the code was not using pathlib at all (everything was string-based). So I made the change gradually: my first step was to introduce pathlib and object-oriented gallery descriptions, but not to change much the execution part (that required strings).
A review would definitely be neccessary but since the tests have not been migrated yet (#12 ), this is a bit risky for now.

Thanks for #39 , I will have a look !

[smarie]

Fixed by #39