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

Set the verbose option for the tests #20

Closed
wants to merge 1 commit into from

Conversation

seb128
Copy link
Contributor

@seb128 seb128 commented Feb 9, 2024

Otherwise they don't display anything which is confusing

Otherwise they don't display anything which is confusing
@seb128
Copy link
Contributor Author

seb128 commented Feb 9, 2024

I was looking at the Debian/Ubuntu build log and the default output doesn't display any summary of what tests were run so I'm adding the -v option to our packaging, it might also make sense upstream?
An alternative option would be to make the non verbose mode at least display a summary of tests successed/skipped/ignored/failed

@mike-fabian
Copy link
Owner

They display something when something fails, isn‘t that enough?

When they display a lot of stuff always, it is much harder to see the failures.

@seb128
Copy link
Contributor Author

seb128 commented Feb 12, 2024

The reason I proposed the change is that by looking at the build log it was not obvious that any test was run

Example from https://launchpadlibrarian.net/711162924/buildlog_ubuntu-noble-amd64.langtable_0.0.64-2_BUILDING.txt.gz

   debian/rules execute_after_dh_auto_test
make[1]: Entering directory '/<<PKGBUILDDIR>>'
cd langtable &&	python3 langtable.py
python3 test_cases.py
python3 main.py
make[1]: Leaving directory '/<<PKGBUILDDIR>>'

In fact the 'python3 main.py' was indeed not working but that was not even visible from the log (the code check for files in '/local/mfabian/src/cldr/common/main' which isn't available on the build system)

(I've changed the package now to call 'make test' instead of individual commands)

You are right that it is verbose, what about displaying the summary in non verbose mode?
For example langtable.py -v displays those lines at the end which could also be displayed without the -v option?

128 tests in 89 items.
128 passed and 0 failed.

@mike-fabian
Copy link
Owner

When a test fails, I want to see in a concise output what failed. Like this:


$ make test
python3 langtable/langtable.py
**********************************************************************
File "/local/mfabian/src/langtable/langtable/langtable.py", line 1056, in __main__.parse_locale
Failed example:
    parse_locale('de_DE')
Expected:
    Locale(language='xde', script='', territory='DE', variant='', encoding='')
Got:
    Locale(language='de', script='', territory='DE', variant='', encoding='')
**********************************************************************
1 items had failures:
   1 of  27 in __main__.parse_locale
***Test Failed*** 1 failures.

It should not be hidden in a lot of useless junk.

Therefore I don't like adding the -v option.

I can make the output a bit more verbose if everything passes though.

How about printing this when all tests pass?:

747 tests run. 747 passed and 0 failed.
Test passed.

@mike-fabian
Copy link
Owner

You are right that it is verbose, what about displaying the summary in non verbose mode?
For example langtable.py -v displays those lines at the end which could also be displayed without the -v option?

Yes, I am no doing that... ⏳

mike-fabian pushed a commit that referenced this pull request Feb 13, 2024
Resolves: #20

Thanks to Sebastian <seb128@ubuntu.com> for the pull request.
@mike-fabian
Copy link
Owner

With this change, 6328d06
the output when all tests pass looks like this:

mfabian@hathi:/local/mfabian/src/langtable (release-candidate-0.0.66)
$ make check 
python3 langtable/langtable.py
128 tests run. 128 passed and 0 failed.
All tests passed.
python3 test_cases.py
747 tests run. 747 passed and 0 failed.
All tests passed.
xmllint --noout --relaxng langtable/schemas/keyboards.rng langtable/data/keyboards.xml.gz
langtable/data/keyboards.xml.gz validates
xmllint --noout --relaxng langtable/schemas/languages.rng langtable/data/languages.xml.gz
langtable/data/languages.xml.gz validates
xmllint --noout --relaxng langtable/schemas/territories.rng langtable/data/territories.xml.gz
langtable/data/territories.xml.gz validates
xmllint --noout --relaxng langtable/schemas/timezones.rng langtable/data/timezones.xml.gz
langtable/data/timezones.xml.gz validates
xmllint --noout --relaxng langtable/schemas/timezoneidparts.rng langtable/data/timezoneidparts.xml.gz
langtable/data/timezoneidparts.xml.gz validates
mfabian@hathi:/local/mfabian/src/langtable (release-candidate-0.0.66)
$ 

The test python3 langtable/langtable.py also exists with a non-zero exit code now when tests fail, it did always exist with 0 before.

@mike-fabian
Copy link
Owner

Closing because I think this is solved.

@mike-fabian mike-fabian added this to To do in Mike’s github project board via automation Feb 13, 2024
@mike-fabian mike-fabian self-assigned this Feb 13, 2024
@mike-fabian mike-fabian moved this from To do to Done in Mike’s github project board Feb 13, 2024
@mike-fabian
Copy link
Owner

mike-fabian commented Feb 13, 2024

In fact the 'python3 main.py' was indeed not working

Ah, by the way, python3 main.py is not a test case. I should probably not have called this main.py.

I use this main.py to get new translations and translations changes from CLDR. It is only partially automated at the moment.

There might be translation changes I do not want for whatever reason and need to override. So far this has never happened, until now I have always taken all translations from CLDR unchanged. So I could probably automate this a bit more and rename main.py to something reasonable, the name should tell what it is used for. Maybe get_translation_updates_from_cldr.py?

@seb128
Copy link
Contributor Author

seb128 commented Feb 13, 2024

Thanks for the formatting changes, that looks great!

And yeah, renaming the script would probably make sense

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants