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

Basic implementation for exported_context. #129

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Askir
Copy link

@Askir Askir commented Feb 4, 2019

Alright as the title suggests this implementation has quite a few limitations. But I hope I can get some feedback on how to go from here.

Limitations are:

  • Exported contexts only work if they are loaded before included.
  • Exported contexts can not include each other (although I am not sure if that works with shared_contexts in general)

I've added pending tests for both scenarios.

Once finished this should implement #128

@kfischer-okarin
Copy link
Contributor

Looks like a good start to me, though from my point of view there is still a bit of work to do before this will work

  • I think you need to add an preliminary run to the ExampleCollector through the files that just returns the exported contexts to fix the loading order problem

  • Does this work if the exported context references a namespace element that does not exist in the imported context? I have the feeling it won't unless you keep the whole parsed modules from the preliminary run and not only the node body...

    #------------- export_spec.py -------------
    def local_function():
          # do something
    
    with exported_context('Exported'):
        with it('tests something'):
            local_function() 
    
    #------------- import_spec.py -------------
    with included_context('Exported'):
        pass

@Askir
Copy link
Author

Askir commented Feb 5, 2019

@DerTraveler
Pretty certain that wont work, good catch!
So you suggest I implement a collect_exported_contexts() method to scan all the modules for exported contexts first?
That would fix the load order limitation but wont fix contexts including each other and also will not fix the namespace problem.

As an idea: If I save the filepath for each exported context, when including that context I could programmatically add an import statement for that file.

On a more general note: I'm a little bit surprised that mamba is implemented with custom ast parsing. I am pretty sure that RSPEC for example is not implemented with custom parsing. And I thought by using pythons with clause mamba would have an easier time to do things like this as we wouldn't be having to re-implement core python functionality like imports.

Is there a specific reason for this? Is it easier to write the custom parser than for example, creating shared_contexts with the method call shared_context() and saving them in a singleton-like structure? Not sure if you are the correct person to ask for a decision like this :D

@maximderbin
Copy link

maximderbin commented Sep 8, 2020

any progress on it? or this PR is dead?

@Askir
Copy link
Author

Askir commented Sep 8, 2020

@maximderbin

any progress on it? or this PR is dead?

I'm still alive but the PR is pretty dead 😅. Haven't worked on this in years basically.

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

Successfully merging this pull request may close these issues.

3 participants