python / cpython

The Python programming language
https://www.python.org
Other
63.61k stars 30.47k forks source link

Idle: test configuration files #65895

Open terryjreedy opened 10 years ago

terryjreedy commented 10 years ago
BPO 21696
Nosy @terryjreedy
Files
  • test-configuration.diff
  • test-configuration-v2.diff: Update patch as per Rietveld comments.
  • Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

    Show more details

    GitHub fields: ```python assignee = 'https://github.com/terryjreedy' closed_at = None created_at = labels = ['3.8', 'expert-IDLE', 'type-feature', '3.7'] title = 'Idle: test configuration files' updated_at = user = 'https://github.com/terryjreedy' ``` bugs.python.org fields: ```python activity = actor = 'terry.reedy' assignee = 'terry.reedy' closed = False closed_date = None closer = None components = ['IDLE'] creation = creator = 'terry.reedy' dependencies = [] files = ['35570', '35641'] hgrepos = [] issue_num = 21696 keywords = ['patch'] message_count = 6.0 messages = ['220053', '220225', '220271', '220514', '228258', '360837'] nosy_count = 3.0 nosy_names = ['terry.reedy', 'jesstess', 'Saimadhav.Heblikar'] pr_nums = [] priority = 'normal' resolution = None stage = 'needs patch' status = 'open' superseder = None type = 'enhancement' url = 'https://bugs.python.org/issue21696' versions = ['Python 3.7', 'Python 3.8'] ```

    terryjreedy commented 10 years ago

    Spinoff of bpo-12274 and a dependency thereof: new test_configurations.py should statically test that configparser, called from idlelib.configHandler, can process all the configuration.def files without error.

    terryjreedy commented 10 years ago

    The format paragraph entry in main.def is being moved to extensions.def, so don't test for it in main.def.

    22200024-de1a-4081-ad85-2ac04e6b54d2 commented 10 years ago

    Attaching a patch to test the default configuration files. config-keys.def will be added once the issues related to it[1] are resolved. In this patch, test that the configHandler module can successfully extract the values. For places where numeric values are expected, ensure that the values are correct. For strings, it only ensures that the values are reasonably correct.

    ----

    [1] https://mail.python.org/pipermail/idle-dev/2014-June/003431.html

    taleinat commented 10 years ago

    I've done a thorough review of the tests. The overall structure and tests are fine, but I found quite a few issues that should be addressed.

    Saimadhav, considering these are some of the first tests you've written, please know that I am impressed! I have many comments because I believe our tests should be excellent and I'm a bit pedantic. However I've seen many people write poorer test suites after despite having considerable experience. So keep up the good work, learn from our feedback and everything will be great!

    terryjreedy commented 10 years ago

    I closed bpo-12274, but it might be consulted before closing this.

    terryjreedy commented 4 years ago

    The issue is obsolete with respect to the minimal test implied by the opening message. At least two tests (added since this was written) read the .def files.

    Some of the proposed tests are, now at least, redundant For boolean tests like self.assertIn(get('editor-on-startup', 'bool'), (True, False)) a non-boolean value results in a Warning (treated like error), the assert, and remaining tests are skipped, and IDLD hangs. Stripping strings should not be needed.

    Some of the tests should be part of a startup check of default *and* user options. Values should also be checked when changed (they mostly are).

    Since new options have been added, some tests are missing.

    I would like to have a 3-layer option map that can be used for both runtime change and the test suite.