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

Provide a generic test suite for MatrixObjects #5162

Draft
wants to merge 19 commits into
base: master
Choose a base branch
from

Conversation

wucas
Copy link
Contributor

@wucas wucas commented Oct 21, 2022

This PR resolves Issue #4512.

Still missing:

  • Comments
  • More tests
  • ability to test wrong inputs, i.e. situations when it is expected and correct that an error is thrown
  • Documentation
  • More options

At some point we should rename the file and do a rebase of course as well.

@wucas wucas added gapdays2022-summer Issues and PRs that arose at https://www.gapdays.de/gapdays2022-summer topic: tests issues or PRs related to tests release notes: to be added PRs introducing changes that should be (but have not yet been) mentioned in the release notes labels Oct 21, 2022
tst/testinstall/MatrixObj/testmatobj_new.g Outdated Show resolved Hide resolved
tst/testinstall/MatrixObj/testmatobj_new.g Outdated Show resolved Hide resolved
tst/testinstall/MatrixObj/testmatobj_new.g Outdated Show resolved Hide resolved
tst/testinstall/MatrixObj/testmatobj_new.g Show resolved Hide resolved
lib/matobjminimal_BROKEN.gi Show resolved Hide resolved
lib/matobjminimal_BROKEN.gi Show resolved Hide resolved
# change an object and therefore require it to be mutable are skipped. This
# option is intended for objects which are always immutable.

TestMatrixObj := function(filter, optIn)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What calls TestMatrixObj and where?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That is the function one should call to execute the tests.

tst/testinstall/MatrixObj/testmatobj_new.g Outdated Show resolved Hide resolved
tst/testinstall/MatrixObj/testmatobj_new.g Outdated Show resolved Hide resolved
# See TestMatrixObj_GenerateExample
# dimensions:
# See TestMatrixObj_GenerateExample
# dimensions_red: A list of tuples defining dimensions for matrices. If not set
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
# dimensions_red: A list of tuples defining dimensions for matrices. If not set
# dimensions_red:
# A list of tuples defining dimensions for matrices. If not set

Instead of "If not set" here and "By default this is set" elsewhere and so on, I'd suggest to settle on a uniform way to describe defaults. E.g. at the end of a description, add, in a line of its own:

#   Default value: XYZ

# domains_red:
# A list of domains in which matrices should be generated. By default
# this is set to [Integers, Rationals, GF(2), GF(5)]. These domains are used
# to generate examples for more expensive tests.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Based on what you write elsewhere, I assume "red" stands for "reduced" and not the color "red"? But why "reduced" ?

It might also be good to explain how this relates to domains. E.g. is this right? "If domains_red is given, then it is used instead of domains for certain expensive tests.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I chose 'reduced' since its an option to choose a smaller set of cases to test. There might be a better name though.

# domains and dimensions via domains_red and dimensions_red all combinations
# of the specified dimensions and domains are used. If tests are very
# expensive or just certain combinations should be tested one can use
# reduced_cases instead.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This option is documented but does not seem to actually exist?

tst/testinstall/MatrixObj/testmatobj_new.g Outdated Show resolved Hide resolved
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
gapdays2022-summer Issues and PRs that arose at https://www.gapdays.de/gapdays2022-summer release notes: to be added PRs introducing changes that should be (but have not yet been) mentioned in the release notes topic: tests issues or PRs related to tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants