Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

Use a regular HomeServerConfig object for unit tests #4889

Merged
merged 8 commits into from
Mar 19, 2019

Conversation

richvdh
Copy link
Member

@richvdh richvdh commented Mar 19, 2019

Rather than using a Mock for the homeserver config, use a genuine HomeServerConfig object. This makes for a more realistic test, and means that we don't have to keep remembering to add things to the mock config every time we add a new config setting.

It even found a bug (cf #4888, on which this PR is based).

Fixes a bug where hs_disabled_message was not enforced for 3pid-based requests
if there was no server_notices_mxid configured.
@richvdh richvdh requested a review from a team March 19, 2019 11:38
richvdh added 6 commits March 19, 2019 11:43
The Mailer expects the config object to have `email_smtp_pass` and
`email_riot_base_url` attributes (and it won't by default, because the default
config impl doesn't set any of the attributes unless email_enable_notifs is
set).
* Set allow_guest_access = True, since we rely on it
* config doesn't have a `hostname` attribute; it is `server_name`
Make sure that we have a `server_notices_mxid` set, given that we are relying
on it.
Rather than using a Mock for the homeserver config, use a genuine
HomeServerConfig object. This makes for a more realistic test, and means that
we don't have to keep remembering to add things to the mock config every time
we add a new config setting.
@richvdh richvdh force-pushed the rav/test_real_config branch from 29aefd2 to 07f057a Compare March 19, 2019 11:44
@codecov
Copy link

codecov bot commented Mar 19, 2019

Codecov Report

❗ No coverage uploaded for pull request base (develop@88f0675). Click here to learn what that means.
The diff coverage is 25%.

@@            Coverage Diff             @@
##             develop    #4889   +/-   ##
==========================================
  Coverage           ?   73.34%           
==========================================
  Files              ?      326           
  Lines              ?    33953           
  Branches           ?     5600           
==========================================
  Hits               ?    24902           
  Misses             ?     7410           
  Partials           ?     1641

@codecov
Copy link

codecov bot commented Mar 19, 2019

Codecov Report

Merging #4889 into develop will increase coverage by 0.03%.
The diff coverage is 100%.

@@             Coverage Diff             @@
##           develop    #4889      +/-   ##
===========================================
+ Coverage     77.9%   77.93%   +0.03%     
===========================================
  Files          326      326              
  Lines        33952    33958       +6     
  Branches      5598     5601       +3     
===========================================
+ Hits         26449    26464      +15     
+ Misses        5885     5872      -13     
- Partials      1618     1622       +4

turns out this relies on there being a `user_consent_version` set.
@richvdh richvdh merged commit 5cf00c9 into develop Mar 19, 2019
@richvdh richvdh deleted the rav/test_real_config branch March 19, 2019 12:27
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants