Skip to content
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

Directory structure #7

Closed
milancurcic opened this issue Dec 14, 2019 · 20 comments
Closed

Directory structure #7

milancurcic opened this issue Dec 14, 2019 · 20 comments

Comments

@milancurcic
Copy link
Member

Mostly cosmetic, but affects quality of life: What kind of directory structure should we use for the library?

Potentially touching on #2 and #3.

I tend to like structures like this:

stdlib/
  src/
    lib/
      mod_stdlib.f90
      subdir1/
      subdir2/
      ...
    tests/
@zbeekman
Copy link
Member

Tests

I really think that it's important to make it easy for people to write and add tests, preferably before they write the implementation.

In my ZstdFortranLib project adding tests for a source file that makes up a library or executable target is as simple as adding a corresponding file with the same name, but ending in _test in the test directory. I have some CMake logic that searches through all Fortran sources associated with a target and looks for corresponding test files, and then automatically adds them. In this way, adding tests for a new source file is easier (from the build system perspective) than adding the actual source file is.

.e.g., if foo.F90 is used in libfoo then putting foo_test.F90 in the test subdirectory automatically creates the test executable, links it to libfoo and adds the CMake test.

Flat is better than nested

In general, I think we should strive for a directory structure that is as flat as possible, and only start adding subdirectories when we reach some threshold of 3-4 files implementing a well defined, cohesive idea. I also generally like having a test or tests directory in each source directory, i.e., lib/tests, subdir1/tests, etc too because it narrows the scope of looking for a test that corresponds to a source file. (Or even just putting tests along side with the _test.[Ff]90 ending in the same directory until it gets too cluttered to be sustainable.)

@certik
Copy link
Member

certik commented Dec 17, 2019

I agree with @zbeekman to have tests and more flat structure. I usually name tests as test_*.f90.

@milancurcic
Copy link
Member Author

Great, I agree, I like the flat structure as well. I'd caution against keeping test source files together with module files at first. We don't have any modules now, but where we want to arrive is a place with a ~dozen to a few dozen modules. In a flat directory structure, it'd soon become cluttered to keep test programs in the same directory.

Less important and personal preference, but when I explore libraries, I first like to look in the library source directory and see what does this library have unless already covered in the README.md. Test files would get in the way of readability here. We should think of the first impression that a new user gets when landing onto the repo page.

@certik
Copy link
Member

certik commented Dec 17, 2019

I think @zbeekman is suggesting to keep tests in the tests subdirectory. I think we all agree on that one, so that the tests do not clutter the source files.

@milancurcic
Copy link
Member Author

Yes, I definitely misread that. :) We're on the same page.

@zbeekman
Copy link
Member

Yes, I'd recommend putting tests in a test or tests subdirectory of the directory that holds the sources to be tested: E.g., src/lib/subdirA/tests for unit tests and specs corresponding to files in src/lib/subdirA. At least for unit tests and specs this makes sense. For integration tests or any test that doesn't have an obvious one-to-one mapping a test or tests directory at the project root (or directly under src if src/lib is where the library code goes) is needed to hold them.

I'm not sure I see the benefit of src/lib/... over just src/ unless we want to pack, e.g., development scripts, etc. into src/, but I'm open to being convinced/wrong. 😉

@milancurcic
Copy link
Member Author

Great, so if we have tests/ in each sub-directory, then I agree that src/lib is redundant, and go with just src/. Given this, how would you organize tests for modules that don't have subdirectories, for example, if our first implementation module is ascii.f90 from #11, do we have:

src/
  stdlib.f90
  ascii.f90
  tests/
    test_ascii.f90

@zbeekman
Copy link
Member

zbeekman commented Dec 18, 2019 via email

@certik
Copy link
Member

certik commented Dec 18, 2019

@milancurcic that's precisely how I would do it.

@zbeekman
Copy link
Member

Is this finalized? Should I work on CMake helper functions to detect and add tests?

i.e. src/ascii.f90 --> src/tests/test_ascii.f90?

@certik
Copy link
Member

certik commented Dec 22, 2019

Isn't it better to list tests explicitly? I feel the automatic detection requires programming in CMake and I would prefer to avoid as much programming as possible in CMake, because CMake is not a very good language for programming. Also, I think a lot of people asked us to also maintain manual Makefiles. Which is quite easy if we keep CMake programming to minimum.

@zbeekman
Copy link
Member

Sure, we can take either approach, but automatic addition of tests is really nice for contributors because THEY don't have to program in CMake... They just create a test and it gets added and run. Otherwise they need to call add_executable add_test target_link_libraries etc etc. With automatic detection creating a file with the right name in the right place causes the test to get added, compiled, linked and run...

@zbeekman
Copy link
Member

I personally think this is worth the CMake "headache" and I am happy to maintain and create it...

@milancurcic
Copy link
Member Author

They just create a test and it gets added and run. Otherwise they need to call add_executable add_test target_link_libraries etc etc.

If done in a loop, it's really just typing out the test name, which is minimal work.

I recommend against depending on a "key man" for any specific task, for example Zaak's CMake expertise. If we keep the workflow simple enough, a developer can (and should) work through the whole cycle: Writing code and tests, documentation, and adding to build systems. On one hand, automating things like adding tests to CMake will help every developer. On the other hand, everybody having to "touch" each part of the workflow will help them be more versed in the whole workflow. In a nutshell, growing a community of generalists rather than that of specialists.

The question of maintaining manual Makefiles raises a deeper question: How do we maintain build systems that we agree on? I see the value in manual Makefiles, but we would as a community have to agree that everybody who contributes a feature also has to contribute the changes to all build systems that we agree to maintain. Specifically, we can't have one person running around patching up manual Makefiles while other developers are rapidly adding and updating the API.

Should we discuss maintaining CMake and manual Makefiles in separate issues (they're off-topic here)?

@milancurcic
Copy link
Member Author

I realize that there will be some balance here. There will likely be some contributors who will implement a feature but won't know how to update build systems and will need help. Expecting them to learn the whole flow and knowing how to update build systems may slow down implementations moving forward. I don't know what's a good approach here.

@zbeekman
Copy link
Member

I recommend against depending on a "key man" for any specific task, for example Zaak's CMake expertise.

I definitely agree with the sentiment here. We don't want our "bus factor" to be low. (I.e., how many people getting hit by a bus will cause the project problems...)

I think that having the automation of setting up tests will improve the project because people already don't want to write tests. Making the lowest possible barrier to entry, i.e., name it with the same name and prefix test_ and put it in the tests subdirectory will go a long way towards making it easy for people to contribute. Even if they have no CMake experience adding the test to CMake makes it so that they don't need to do anything... It's not THAT hard to loop over a target's source files and look for ones that correspond to tests. If the CMake is well commented even relative novices should be able to follow along.

Even if adding tests is automated, we should document manually adding tests; sometimes you need to do this. We can have examples of manual tests too. However, I think that the automation and ease of use make it worthwhile to have automated finding and running of tests. The build system could even warn you if you add a new source file without a corresponding test.

I know CMake can be awkward, and scary, but creating this will ultimately lead to fewer lines of CMake code and should make peoples lives easier, and lead to more tests being submitted with PRs.

@certik
Copy link
Member

certik commented Dec 22, 2019 via email

@certik
Copy link
Member

certik commented Dec 22, 2019 via email

@jvdp1
Copy link
Member

jvdp1 commented Dec 27, 2020

As noticed by @LKedward here and mentioned in #279, the current directory structure of stdlib is different than the directory structure required by fpm. I think it could be strange for new users to find that the directory structure of stdlib is different than the one of fpm Therefore it could be good to revise the directorty stucture of stdlib. It would also help its support by fpm.

@milancurcic
Copy link
Member Author

I agree, let's pursue the changes needed in #279. The pre-fpm directory structure has been established and doing its job well for a while, and a new structure will be tackled in the specific issue above. So I will close this.

jvdp1 added a commit that referenced this issue Dec 26, 2023
Changes:

* Windows: GCC 13
* Ubuntu-latest: Intel LLVM (instead of Classic)
* Relax test in test_rawmoment due to change from Intel Classic to Intel LLVM
jvdp1 added a commit that referenced this issue Dec 26, 2023
Fix ci for windows and Intel ifx (#7)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants