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

Consider not being so strict about ftltest context option ordering #1866

Closed
matt2e opened this issue Jun 24, 2024 · 1 comment · Fixed by #2018
Closed

Consider not being so strict about ftltest context option ordering #1866

matt2e opened this issue Jun 24, 2024 · 1 comment · Fixed by #2018
Assignees
Labels

Comments

@matt2e
Copy link
Collaborator

matt2e commented Jun 24, 2024

  • Currently we build the context in order that the options were provided
  • This has caused confusion, such as with this example where it was not obvious why it didn't work:
func newContext() (context.Context, *_dal.DAL) {
	ctx := ftltest.Context(
		ftltest.WithMapsAllowed(),
		ftltest.WithDatabase(dbHandle), // <--- Assumes we have DSNs
		ftltest.WithDefaultProjectFile(), // <--- Provides DSNs
	)

	dal := _dal.New(dbHandle.Get(ctx))

	return ctx, dal
}

We may want to always handle project file options first, which would solve this issue.

Alternatively we could detect that the project file option was provided after options that could make use of it so atleast we explain to the user what to change.

@github-actions github-actions bot added the triage Issue needs triaging label Jun 24, 2024
@ftl-robot ftl-robot mentioned this issue Jun 24, 2024
@alecthomas
Copy link
Collaborator

A common pattern is for the options to just populate an intermediate structure with data, then an initialisation phase actually constructs everything.

@matt2e matt2e added the next Work that will be be picked up next label Jul 1, 2024
@github-actions github-actions bot removed the triage Issue needs triaging label Jul 1, 2024
@matt2e matt2e added the P1 label Jul 1, 2024
@jonathanj-square jonathanj-square self-assigned this Jul 2, 2024
@github-actions github-actions bot removed the next Work that will be be picked up next label Jul 2, 2024
jonathanj-square added a commit that referenced this issue Jul 10, 2024
fixes #1866 

introduces a rudimentary partial ordering scheme for context options -
this is used to sort them before they get executed to build the context
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants