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

Adding a mechanism to regenerate expected values #35

Open
wants to merge 9 commits into
base: master
Choose a base branch
from

Conversation

mmesiti
Copy link
Collaborator

@mmesiti mmesiti commented Oct 9, 2024

From #34 :

I think it would be best to make it easier to regenerate the results, so that the next time this step can be automated.

Creating this PR for feedback.

Notable things:

  • if the environment variable "PMFRG_REGEN_EXPECTED_RESULTS" is set to any value, then it creates
    • a (quasi-) drop-in replacement of ExampleObservables.jl, that contains:
    • the date of the generation
    • information about the packages and their dependencies (dumped from Pkg.project() and Pkg.dependencies()) (a bit overkill, perhaps, but part of the dump is surely useful)
    • the commit hash of HEAD when running the tests
    • the SHA-1 of Manifest.toml when running the tests (from git hash-object Manifest.toml)
    • new definitions for each of the methods of example_Obs that has been called during the test run.
    • a copy of the current Manifest.toml that will be added (but not committed) to the git repository.
  • at the moment the versions are pinned to the known working ones with [compat].

Things done:

  • test that the output can be used as a drop-in replacement after calling format() on it (see last commit)
  • document this in the README
  • removed the [compat] constraints

Things still to do, possibly:

  • proper names and code organization (I am more or less happy with that)
  • perhaps clean the dump of Pkg.project() and Pkg.dependencies() ? No, using Pkg.project() and Pkg.dependencies() is actually a bad idea because the Pkg API is not stable and can break between Julia versions. Manifest.toml is a better tool for the job, because it allow format changes (there's a manifest_format field for the version for this).
  • auto-format the output. I think this can still be done manually, I prefer avoiding adding JuliaFormatter as yet another dependency in the [extras] - unless you are against it.

This PR does the following:

  • it puts in the logic to update them quickly.
  • in the last commit, it removes the [compat] constraints from Project.toml and updates the expected values.

@mmesiti mmesiti marked this pull request as ready for review October 30, 2024 08:42
Michele Mesiti added 6 commits October 30, 2024 11:01
The output values computed in the tests,
and compared to the expected results,
can be regenerated by setting the environment variable
"PMFRG_REGEN_EXPECTED_RESULTS" to any value
when running the tests.
and some tests for readme
- Missing `using Pkg` at the beginning now added
- Using type instead of value
  when creating new methods for example_Obs
Michele Mesiti added 3 commits November 5, 2024 14:59
The project/deps objects
that are created at the beginning
of the automatically generated ExampleObservable.jl file
belong to the Pkg API
which is not very stable,
so if they are read by different julia version
they might let to a crash of the test suite.

The representation of the proj/deps objects
has been sandwiched in an if statement
that allows the objects to be created
only if the julia VERSION
is the one used when generating the values.

Also, the code has been refactored in smaller functions.
projec/deps objects not create any more in ExampleObservables.jl,
since the Pkg API is unstable.

Making a copy of the Manifest to be stored alongside
ExampleObservables.jl.
Some methods were defined twice
because the regenerated file was not cleaned
before re-running the test suite.

Also, minor tweaks for readability
and to the documentation
(including tests)
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

Successfully merging this pull request may close these issues.

1 participant