sighmon / mjml-rails

MJML + ERb/Haml/Slim view template
https://mjml.io
Other
296 stars 64 forks source link

Tests fail randomly #61

Closed sighmon closed 3 years ago

sighmon commented 4 years ago

Seems to always fail on https://github.com/sighmon/mjml-rails/blob/master/test/parser_test.rb#L43

expect(Mjml.beautify).must_equal(true)

To replicate:

f-mer commented 4 years ago

Hay there :wave:

The test seems to fail randomly because it is order-dependent. If the "use setup config' test is executed before the "use defaults if no config is set" test it will fail because Mjml.beautify is a global variable and it's not reset to true within the test case.

You can observe this behaviour locally by specifying the seed on test execution: rake test SEED=2185.

Thanks a lot for maintaining this gem :slightly_smiling_face:

sighmon commented 4 years ago

@f-mer That makes sense, thank you! I was thinking of reloading the lib before the failing test, but is there a better way to do that to avoid the warnings about previously defined methods/constants?

      it 'use defaults if no config is set' do
        load 'lib/mjml.rb'
        expect(Mjml.beautify).must_equal(true)
        expect(Mjml.minify).must_equal(false)
        expect(Mjml.validation_level).must_equal('soft')
      end

Outputs:

...warning: method redefined; discarding old template_language
f-mer commented 3 years ago

I think a good way to go about this is to move configuration into its own class. I've opened a merge request :slightly_smiling_face:

In the current state it breaks backwards compatibility but we could delegate all properties to the configuration object :thinking: But I think a better way would be dropping direct access to configuration properties via Mjml.

Instead of reloading the whole library the configuration is reset between the test cases Mjml.configuration = nil.